mars-nwe/REDESIGN.md

# mars-nwe NCP dispatch redesign notes

This file collects design notes for a possible cleanup of the internal NCP
handoff path.  It is intentionally separate from `TODO.md`: the TODO file should
track concrete bugs and endpoint audit follow-ups, while this file describes a
larger architecture direction that can be implemented gradually.

The goal is not to rewrite MARS-NWE at once.  The goal is to make the current
handoff behavior explicit, reduce ambiguity around magic return values, and make
future endpoint work easier to audit against the Novell/Micro Focus SDK, WebSDK,
and NDK Core Protocols PDF.

## Current problem

The current NCP path grew around several cooperating processes and handlers:

- `nwconn.c` owns the connection/session side and receives most packets first.
- `nwbind.c` handles bindery, queue, some server-management, and some final
  reply construction.
- Other modules such as semaphore, message, namespace, AFP, file, salvage, and
  queue code implement individual protocol families or backend actions.
- Some calls are handled completely in `nwconn.c`.
- Some calls are forwarded to `nwbind.c` by returning `-1` from the `nwconn.c`
  dispatcher.
- Some calls are forwarded with saved request state by returning `-2`, so that
  `nwconn.c` can do post-processing after `nwbind.c` has replied.
- Some forwarded paths mutate request payloads before handoff.
- Some code paths build responses locally, while other paths rely on the target
  process to build the final completion code and payload.

This works, but it is hard to reason about while auditing endpoint layouts.  The
same looking value can mean different things depending on which file it appears
in.  For example, `return(-1)` in the relevant `nwconn.c` dispatcher path means
"forward this request to `nwbind`".  A disabled `return(-1)` inside a `#if 0`
block in `nwbind.c` does not have that forwarding meaning and should not be
copied into active code.

The visible symptoms are:

- endpoint documentation must follow a handoff across files before it can say the
  request or reply layout is known;
- missing endpoints are difficult to distinguish from forwarded endpoints;
- request parsing, backend behavior, reply encoding, and process routing are
  often mixed in one switch block;
- byte order differences are easy to miss because parsing and reply writing are
  open-coded in different places;
- disabled future stubs can look like active dispatch behavior;
- `TODO.md` can become a dumping ground for architectural observations that are
  not immediate endpoint bugs.

## Desired shape

A cleaner long-term structure would have one small internal NCP dispatch layer:

```text
wire packet
  -> NCP envelope parser
  -> NcpContext
  -> endpoint lookup
  -> endpoint handler / provider
  -> reply encoder
  -> central reply sender
```

This does not need to be a general-purpose message bus.  A full message bus would
probably be too large and too abstract for this code base.  A typed internal NCP
context plus explicit dispatch results would be enough.

The important separation is:

1. decode the packet envelope;
2. identify the endpoint;
3. decode the endpoint request body;
4. execute the backend operation;
5. encode the endpoint reply body;
6. send the response from one well-defined place.

## Proposed NCP context

Introduce, in a later functional cleanup, a small context object that represents
one NCP request while it moves through the server.  The exact field names should
fit the existing code style, but the conceptual shape would be:

```c
typedef struct {
  int connection;

  uint16_t request_type;        /* 0x2222, 0x3333, 0x5555, ... */
  uint8_t function;             /* top-level NCP function */
  int has_subfunction;
  uint8_t subfunction;          /* grouped calls such as 23/x, 32/x, 87/x */

  const uint8_t *request;
  int request_len;

  uint8_t *reply;
  int reply_cap;
  int reply_len;

  uint8_t completion;
  uint8_t connection_status;

  uint32_t flags;
} NcpContext;
```

The context should not replace all old globals in one patch.  It can start as a
thin wrapper around the existing request and response buffers, then gradually
become the preferred handler interface.

The useful property is that endpoint documentation can point to a stable model:

- `function` and `subfunction` identify the endpoint;
- `request` and `request_len` are the bytes after the already-decoded envelope;
- `reply` and `reply_len` are the bytes before the common NCP response envelope;
- `completion` is set once by the handler or by central error handling.

## Replace magic return values with named results

The current `0`, `-1`, and `-2` convention should be made explicit before any
larger refactor.  The first step can be documentation-only or macro-only:

```c
#define NCP_LOCAL_DONE             0
#define NCP_FORWARD_NWBIND        -1
#define NCP_FORWARD_NWBIND_POST   -2
```

A later cleanup can replace those with an enum:

```c
typedef enum {
  NCP_DISPATCH_DONE,
  NCP_DISPATCH_FORWARD_BIND,
  NCP_DISPATCH_FORWARD_BIND_POST,
  NCP_DISPATCH_NOT_IMPLEMENTED,
  NCP_DISPATCH_BAD_REQUEST,
  NCP_DISPATCH_INTERNAL_ERROR
} NcpDispatchResult;
```

The important rule is that the meaning must be scoped.  A named result returned
from a `nwconn.c` dispatcher may request process handoff.  A return statement in
`nwbind.c` should not silently inherit that meaning unless the function is
explicitly part of the same dispatch interface.

## Endpoint table as audit index first

Before replacing switch statements, add an endpoint inventory table as a
non-invasive audit aid.  It can be compiled only for debug builds or kept as a
source-level documentation table.

Conceptual form:

```c
typedef struct {
  uint16_t request_type;
  uint8_t function;
  int has_subfunction;
  uint8_t subfunction;
  const char *name;
  const char *provider;
  uint32_t flags;
} NcpEndpointDoc;
```

Example entries:

```c
{ 0x2222, 23, 1, 109, "Change Queue Job Entry old", "nwbind/queue", NCPDOC_FORWARDED },
{ 0x2222, 32, 1,   0, "Open Semaphore old",         "sema",         NCPDOC_LOCAL },
{ 0x2222, 33, 0,   0, "Negotiate Buffer Size",      "nwconn",       NCPDOC_LOCAL },
```

This table would help with the ongoing endpoint audit:

- SDK/PDF/WebSDK listed and implemented;
- SDK/PDF/WebSDK listed and forwarded;
- SDK/PDF/WebSDK listed but disabled as a future stub;
- SDK/PDF/WebSDK listed but absent from the current compatibility target;
- later NetWare 4.x/OES/MOAB endpoint, not part of the default NetWare 3.x
  compatibility target.

The first version should not drive runtime dispatch.  It should only make review
and missing-endpoint checks less error-prone.

## Handler structure

For newly touched endpoint families, prefer the following logical split even if
it remains in one C function at first:

```text
request decode
  -> validation
  -> backend operation
  -> reply encode
```

For complex endpoints this could become explicit helper functions:

```c
static int decode_foo(NcpContext *ctx, FooRequest *out);
static int exec_foo(NcpContext *ctx, const FooRequest *req, FooReply *reply);
static void encode_foo(NcpContext *ctx, const FooReply *reply);
```

This is especially useful for endpoint families where the audit has already
found old/new layout differences:

- 16-bit old queue job numbers versus newer 32-bit job numbers;
- big-endian versus little-endian SDK notation;
- old short replies versus newer long replies;
- connection-side prehandling that inserts or rewrites fields;
- bindery or queue paths that build final replies in a different process.

Small endpoints do not need three separate helper functions if that would make
the code noisier.  The rule is that request bytes and reply bytes should be easy
to identify and compare with the SDK documents.

## Make handoff explicit

Forwarded calls should say exactly what is handed off.  A good comment should
answer:

- which bytes are forwarded;
- whether the subfunction byte is preserved or stripped;
- whether `nwconn.c` mutates the request before forwarding;
- whether `nwbind.c` or another provider builds the final reply;
- whether `nwconn.c` expects post-processing after the provider reply.

Examples of handoff cases that need this clarity:

- Queue calls where `nwconn.c` expands paths or inserts job file handles before
  `nwbind.c` sees the request.
- Quota/bindery prehandling where the destination handler receives an already
  transformed request.
- Semaphore and message groups that are grouped in the SDK but routed through
  local helper modules.
- Direct lifecycle calls such as End Of Job and Logout where local cleanup and
  final success reply are split across files.

The preferred future style is not "`nwbind` must do the rest" but something like:

```text
Forward to nwbind with the original subfunction byte and payload unchanged.
No nwconn post-processing is expected; nwbind builds the completion-only reply.
```

or:

```text
Forward to nwbind after saving the original request.  nwbind validates bindery
state and returns the bindery result; nwconn then performs the file-handle
post-processing in handle_after_bind().
```

## Response building rule

Every endpoint audit should identify the reply builder, not only the request
parser.  A handler is not fully documented until the response path is known.

For each endpoint family, record:

- completion-only reply;
- fixed-size payload reply;
- variable-length payload reply;
- provider-built reply;
- `nwconn.c` post-processed reply;
- intentionally unsupported reply status.

Long-term, response sending should become centralized enough that endpoint code
only encodes payload bytes and a completion code.  This reduces off-by-one reply
length bugs and makes the logs easier to normalize.

## Provider boundaries

A clean design would treat the existing modules as providers instead of hidden
fallback paths:

```text
nwconn       connection/session, packet IO, top-level envelope
ncpdispatch  endpoint lookup, handoff policy, common errors
nwbind       bindery database and bindery-backed services
queue        queue metadata and print/backend adapter
sema         semaphore state
message      station/message/broadcast state
namespace    path, directory handle, name-space operations
file         file handle and read/write/open/close operations
salvage      deleted-file scan/recover/purge backend
AFP          AFP metadata and AFP namespace adapter
```

This is a design target, not a demand to move files immediately.  The important
part is that future code should avoid making `nwbind` a catch-all sink for
unrelated NCPs just because it already has an IPC path.


## Provider boundary versus process boundary

A provider boundary is not the same thing as a Unix process boundary.  This is
an important distinction because splitting every NCP family into a separate
process would make the server harder to debug and could introduce new ordering,
locking, and reply-ownership bugs.

The preferred rule is:

```text
first define logical providers;
only later promote the few large stateful providers to separate processes.
```

A logical provider can start as an ordinary C module called from the existing
process path.  It becomes valuable as soon as the dispatch table can say "this
endpoint belongs to the queue provider" or "this endpoint belongs to the
connection-local provider", even if no new process exists yet.  A process split
should be treated as an implementation detail that is only justified when the
provider has enough independent state and lifecycle to benefit from isolation.

This keeps the redesign incremental:

```text
now:
  nwconn switch -> existing local code or nwbind handoff

first cleanup:
  nwconn switch -> provider-named helper/module

later, only where useful:
  nwconn/dispatcher -> IPC -> provider process
```

### Good process candidates

#### Bindery

Bindery is already a natural service boundary.  It owns long-lived server state:
objects, properties, sets, security, password/login/key handling, and object
lookup.  Keeping bindery behind a clear provider boundary is appropriate, and the
existing `nwbind` process can remain that boundary while the dispatch layer is
cleaned up.

The main cleanup is not to remove `nwbind`, but to stop treating it as a generic
catch-all for unrelated forwarded requests.  A future endpoint table should mark
true bindery calls as `bindery`, and queue or management calls should not be
classified as bindery merely because their current implementation lives in
`nwbind.c`.

#### Queue / possible `nwqueue`

Queue management is the strongest candidate for a future separate process after
bindery.  Queue handling has its own domain state:

- queue objects and queue metadata;
- queue job lifecycle;
- queue server attach/detach state;
- service, finish, and abort state;
- job position and priority;
- client-rights transitions during job servicing;
- queue directories and spool/job files.

That is large enough to deserve a logical `queue` provider even before any
runtime split.  A future `nwqueue` process can be considered once request/reply
ownership and bindery access are explicit.

The first step should only be a provider split:

```text
0x2222/23 queue subfunctions -> queue provider
queue provider -> bindery provider/library for object/security/property checks
queue provider -> file/path helpers for queue job files
```

A real `nwqueue` process should not be created by simply moving the current queue
cases out of `nwbind.c`.  It needs an explicit contract for:

- which process owns the final NCP reply;
- how queue calls read bindery objects and properties;
- how queue job files are opened and handed back to the connection process;
- how connection cleanup affects attached queue servers and in-service jobs;
- how old 16-bit job-number calls and newer 32-bit job-number calls are kept
  compatible.

Until those contracts are clear, `nwqueue` should remain a design target, not an
immediate functional change.

### Possible but risky process candidates

#### File and volume subsystem

The file/volume/name-space area is large and stateful, so it can look like a
candidate for a separate process.  It owns or touches directory handles, file
handles, locks, trustee evaluation, volume information, name spaces, salvage and
purge operations, and Unix filesystem mapping.

However, this area is also tightly coupled to connection state and existing file
descriptor ownership.  Moving it behind IPC too early could create more problems
than it solves.  The safer path is:

```text
first:  file/volume/name-space provider modules inside the current process model
later:  consider a process split only after handle ownership is explicit
```

A file provider boundary is useful for documentation and dispatch cleanup.  A
separate file process is optional and should be considered high-risk.

#### Accounting

Accounting is a maybe.  It has a separate protocol domain, but in many setups it
may be small enough to stay as an in-process provider.  A process boundary only
makes sense if accounting grows into a real persistent service with charges,
holds, notes, audit records, and recovery behavior that should be isolated from
connection handlers.

### Poor process candidates

#### Semaphore

Semaphore calls should have a clean provider boundary, but a dedicated process is
probably overkill.  The old semaphore group is small: open, examine, wait,
signal, and close.  It needs shared state, but not necessarily a standalone
process.  A `sema` provider module with clear request/reply ownership should be
enough unless later testing shows that cross-connection semaphore state cannot be
managed safely in the existing process model.

#### Connection lifecycle and session-local calls

Connection lifecycle operations should stay with `nwconn` or a connection-local
provider.  Calls such as Logout, End Of Job, watchdog handling, buffer
negotiation, and connection-state cleanup are fundamentally tied to the session
that received the packet.  Moving them into another process would make cleanup
ordering and error handling harder.

#### Simple server-management calls

Simple management and information calls should not become their own process.
Examples include login-status queries, server description strings, server time,
console-privilege checks, and small broadcast/control helpers.  These can be
represented as a `servermgmt` provider for dispatch clarity, but they should stay
in-process unless a specific call requires an existing backend service.

### Suggested provider map

The endpoint audit table should be able to use provider names like these:

```text
local          packet/session-local handling in nwconn
bindery        object/property/security/login backend
queue          queue objects, jobs, queue servers, spool/job lifecycle
filesystem     file, directory, volume, namespace, trustee, salvage helpers
semaphore      semaphore state and old 0x2222/32 calls
message        station messaging and broadcast helpers
servermgmt     small server-management and information calls
accounting     account status, charges, holds, notes
AFP            AFP namespace and metadata helpers
unknown        documented but not yet mapped
```

Only some providers should ever become processes:

```text
already process-like:  bindery / nwbind
likely future process: queue / possible nwqueue
maybe, high risk:     filesystem
usually in-process:   semaphore, message, servermgmt, accounting, AFP helpers
```

The practical design rule is:

```text
Use provider names everywhere in documentation and endpoint tables.
Use new processes only where shared state, isolation, and lifecycle justify the
extra IPC complexity.
```


## Future NetWare 4.x directory, LDAP, and storage direction

NetWare 4.x support should not be added by letting `nwbind` grow into a second
large catch-all service.  The long-term directory design should keep the legacy
Bindery, the future NDS compatibility layer, and the LDAP protocol frontend as
separate logical layers above one shared directory store.

The intended naming model is:

```text
libflaim
  persistent embedded database engine

directory core/store
  mars-nwe object model, schema, indexes, ACL/auth primitives
  persists its data through libflaim

nwdirectory
  mars-nwe service name for the integrated tinyldap-derived LDAP service
  owns LDAP/LDAPS/StartTLS protocol handling
  uses wolfSSL only at the LDAP network/TLS edge
  calls the directory core/store, not Bindery or NDS packet handlers

nwnds
  future NetWare 4.x/NDS compatibility layer
  owns NDS/NCP directory semantics, contexts, tree-oriented operations,
  NetWare-specific rights/auth behavior, and later compatibility glue
  calls the directory core/store directly

nwbind
  legacy NetWare 2.x/3.x Bindery compatibility layer
  maps Bindery objects, properties, sets, security, and login-visible behavior
  onto the shared directory core/store where possible
```

In this model, `nwdirectory` is not a separate design from tinyldap.  It is the
mars-nwe integration name for the tinyldap-derived LDAP directory service, so
that the installed binary/module follows the existing `nw*` naming scheme.  The
upstream tinyldap code can provide the LDAP protocol implementation, but the
project-facing component should be named `nwdirectory`.

`nwnds` should remain a separate layer because LDAP is only one protocol view of
the directory.  NDS has NetWare-specific semantics that should not be forced into
the LDAP frontend.  Conversely, LDAP clients should not be required to pass
through the NDS/NCP compatibility handler just to reach the directory database.
The preferred relationship is sibling frontends above one core:

```text
                         +----------------------+
                         | directory core/store |
                         | backed by libflaim   |
                         +----------+-----------+
                                    ^
                    +---------------+---------------+
                    |                               |
              nwdirectory                         nwnds
       tinyldap-based LDAP/LDAPS          NetWare 4.x/NDS semantics
       frontend, wolfSSL TLS edge         NCP/NDS compatibility layer
                    ^                               ^
                    |                               |
              LDAP clients                 NetWare/NDS clients
```

The legacy Bindery service should also move toward this shared store over time:

```text
NetWare 3.x client -> Bindery NCP -> nwbind -> directory core/store -> libflaim
LDAP client        -> LDAP/LDAPS -> nwdirectory -> directory core/store -> libflaim
NetWare 4.x client -> NDS/NCP    -> nwnds -> directory core/store -> libflaim
```

That means `nwbind` should become a compatibility mapping over directory objects
and attributes instead of maintaining a completely separate long-term identity
truth.  This is especially important once NetWare 4.x/NDS support exists, because
Bindery compatibility can then be implemented as a legacy view of the same
underlying users, groups, properties, and rights data.

The internal path should not be:

```text
nwbind -> LDAP protocol -> nwdirectory -> directory store
nwnds  -> LDAP protocol -> nwdirectory -> directory store
```

Using LDAP as the mandatory internal storage API would mix protocol concerns into
server internals, make old Bindery behavior harder to preserve, and add needless
encoding/search semantics between tightly coupled modules.  LDAP should remain an
external protocol frontend.  `nwbind`, `nwnds`, and `nwdirectory` should all use a
shared directory-core API or a clearly defined IPC protocol to the directory
store.

FLAIM should therefore be treated as the long-term persistent storage engine for
the directory core, not as an LDAP-only database.  The directory core owns the
schema, object model, indexes, transactions, ACL checks, and authentication
primitives that the protocol/provider layers need.  `nwdirectory` exposes those
objects through LDAP; `nwnds` exposes them through NDS semantics; `nwbind` exposes
them through legacy Bindery calls.

Kerberos should not be part of this initial design.  Classic NetWare 4.x/NDS
compatibility should focus on native NDS-style authentication and directory
semantics.  If a later eDirectory/NMAS compatibility effort ever needs Kerberos,
it should be considered a separate future authentication-provider topic, not a
requirement for the `nwdirectory`/`nwnds`/`nwbind` split.

The migration path should be conservative:

1. add the design boundary and naming notes first;
2. import or integrate tinyldap under the project-facing `nwdirectory` name;
3. keep wolfSSL confined to the LDAP/LDAPS/StartTLS network edge;
4. introduce a directory-core abstraction before making Bindery depend on it;
5. map selected `nwbind` objects/properties to the directory core only after the
   legacy behavior is documented;
6. add `nwnds` later as an NDS semantic layer, not as an LDAP wrapper;
7. only then consider replacing private Bindery persistence with libflaim-backed
   directory storage.

This keeps the future NetWare 4.x work aligned with the provider/process split:
`nwdirectory`, `nwnds`, and `nwbind` may be separate processes or modules, but
they should not be separate sources of truth for identity and directory data.

## Logging connection

The dispatch redesign also supports the desired log cleanup.  If every request
has a context, logs can consistently include:

```text
INFO NCP 23/109 DISPATCH type=0x2222 fn=0x17 sub=0x6d provider=nwbind/queue
INFO NCP 32/0 REPLY type=0x2222 fn=0x20 sub=0x00 result=0x00 len=4
WARN NCP 23/130 LAYOUT-MISMATCH sdk="32-bit JobNumber" code="16-bit parser"
```

The logging cleanup should still reuse existing mars-nwe logging functions.  Do
not add a second logging subsystem just to support the dispatch cleanup.

## Migration plan

### Phase 1: Name the existing conventions

Low risk.  No behavior change.

- Add named constants or comments for the current `0`, `-1`, and `-2`
  dispatcher results.
- Keep existing control flow unchanged.
- Update comments so `return(-1)` is never described ambiguously outside the
  exact dispatcher where it is meaningful.

### Phase 2: Add an endpoint audit table

Low risk.  Mostly documentation/debug.

- Add a table of known endpoints by request type, function, and subfunction.
- Mark provider, generation bucket, and implementation state.
- Use it to compare SDK/PDF/WebSDK coverage against actual handlers.
- Do not switch runtime dispatch to the table yet.

### Phase 3: Introduce a thin `NcpContext`

Moderate risk if kept small.

- Wrap existing request and reply buffers without changing ownership.
- Use the context only in newly audited or newly implemented handlers.
- Keep old handlers callable until they are touched for another reason.

### Phase 4: Convert small endpoint families first

Moderate risk, easy to test.

Good candidates:

- `0x2222/32` old Semaphore calls;
- direct calls such as End Of Job, Logout, and Negotiate Buffer Size;
- small message/station groups once their handoff has been audited.

Avoid converting queue and bindery first because they have more process coupling
and more old/new layout variants.

### Phase 5: Move runtime dispatch to tables gradually

Higher risk.  Do this only after enough endpoint families have stable audit
coverage and tests.

- Keep switch wrappers during the transition.
- Convert one family at a time.
- Preserve exact completion codes and reply lengths.
- Add targeted smoke tests for any family whose dispatch path changes.

## Non-goals

This redesign should not:

- change protocol behavior merely to match a cleaner abstraction;
- remove NetWare 1.x/2.x/3.x compatibility paths;
- enable NetWare 4.x/OES/MOAB-only endpoints by default;
- replace existing mars-nwe path, bindery, queue, AFP, trustee, or salvage
  backends with parallel databases;
- add a large external message bus dependency;
- rewrite all handlers in one patch;
- turn documentation-only endpoint audit patches into functional refactors.

## Practical rule for future patches

For the ongoing endpoint documentation pass, keep doing the conservative thing:

1. enumerate SDK/PDF/WebSDK/include endpoints for the family;
2. compare them with actual `case` labels and forwarded destination handlers;
3. document missing, disabled, implemented, and later-generation slots;
4. document request parser/handoff and response builder;
5. record real layout differences, but do not change behavior in the same patch.

Functional cleanup should come later in small patches with tests.