[Swan-dev] the coding twine holding end's .host_addr, .client, .port, .protocol and .has_client together

[Andrew Cagney]

I just pushed this change into the opportunistic code:

-        * if we have protoport= set, narrow to it and zero out
-        * ephemeral port
-        *
-        * warning: we know ports in this_client/that_client are 0 so
-        * far
-        *
-        * XXX: ?????
+        * Save the selector in .client.
         */
-       if (c->spd.this.protocol != 0) {
-               if (c->spd.this.port != 0) {
-                       update_selector_hport(&c->spd.this.client,
c->spd.this.port);
-               }
-               if (c->spd.that.port != 0) {
-                       update_selector_hport(&c->spd.that.client,
c->spd.that.port);
-               }
-       }
+       c->spd.this.client = local_shunt;
+       c->spd.that.client = remote_shunt;

yet nothing seems to have broken.  Here's my theory as to why.

The end's .client field has been trying to serve two purposes:

- storing the configuration file's subnet
- storing the active connection's subnet/port (protocol came later)

For template connections this works fine: the template's .client
contains the subnet, and the instance's client contains the active
subnet/port.

For non-template connections this doesn't work.  Hence the additional
fields .has_client, .protocol, and .port, .host_addr which all try to
hold things together.  For instance:

- .has_client implies that .client's subnet is valid
even though the actual connection's subnet/port was written into
.client the subnet part is assumed to be unchange

- .protocol+.port (which presumably are never unchanged) are used when
trying to match connections
The .client's port has presumably been scribbled on with the last
active connection's port, maybe.

- strange combinations of (subnetishost(.client) &&
addrinsubnet((.host_addr), (.client)) are used to client's matching
the external interface

Two things might make this easier to understand:

- instantiate non-template connections (this idea keeps coming up on IRC)

- like for ikeports, separately store the original values from the
configuration file