[Swan-dev] pluto: rename and remodularize emit_v2N*

Andrew Cagney andrew.cagney at gmail.com
Thu Jan 3 16:30:05 UTC 2019


On Thu, 3 Jan 2019 at 10:18, Paul Wouters <paul at nohats.ca> wrote:
>
> On Thu, 3 Jan 2019, Andrew Cagney wrote:
>
> >>     pluto: rename and remodularize emit_v2N*
> >
> > Why?   We had emit_v2V() emit_v2N() emit_v2UNKNOWN() et.al.,
now we don't?
>
> The goal was to be able to pass a struct as well so we don't have to
> build up chunks and do any ntoh/hton stuff manually. For example for
> the compression and redirect notify payloads.

But like I pointed out:

> but that could have all been done by adding a static out_v2N(...,
> nested_pbs, ...) and having just the above and emit_v2N() call it?

The compression and redirect payload code could then use that.  No
renaming needed.

</paul at nohats.ca>


> > These assertions were removed.  Why?
> >
> > passert((protoid == PROTO_v2_RESERVED) == (spi == NULL));
> > passert((protoid == PROTO_v2_AH || protoid == PROTO_v2_ESP) == (spi != NULL));
>
> I don't know but some did not make much sense?

Per comment:

        /* See RFC 5996 section 3.10 "Notify Payload" */

here's the text:

   o  Protocol ID (1 octet) - If this notification concerns an existing
      SA whose SPI is given in the SPI field, this field indicates the
      type of that SA.  For notifications concerning Child SAs, this
      field MUST contain either (2) to indicate AH or (3) to indicate
      ESP.  Of the notifications defined in this document, the SPI is
      included only with INVALID_SELECTORS, REKEY_SA, and
      CHILD_SA_NOT_FOUND.  If the SPI field is empty, this field MUST be
      sent as zero and MUST be ignored on receipt.

what in my mind is nonsensical is the check that follows, vis:

                if (protoid == PROTO_v2_RESERVED || spi == NULL) {
                        dbg("XXX: type requires SA; missing");
                }
                break;

if protoid==0 then that debug message is simply bogus.  I added a
comment pointing this out, but it was removed :-(

> >>     - changed emit_* to out_* so uncommited change won't silently fail
> >
> > I've no clue what this even means?
>
> Rename the function so any old callers will break until you fix them up
> to the new function name.

So the "silently fail" was just the build?  I'm not so sure.


>
> Paul


More information about the Swan-dev mailing list