I don't understand this code but I'm "improving" it, based on symmetry.
I've hit a snag.  Advice welcome!

There are four calls to text_said.  Here's what they look like, with
details removed:

        if (endpoint_is_specified(st->st_mobike_local_endpoint)) {
                if (dir == XFRM_POLICY_IN || dir == XFRM_POLICY_FWD) {
                        src = &c->spd.that.host_addr;
                        dst = &c->spd.this.host_addr;
                        set_text_said(n, dst, sa.spi, proto);
                } else {
                        src = &c->spd.this.host_addr;
                        dst = &c->spd.that.host_addr;
                        set_text_said(n, src, sa.spi, proto);
                }
        } else {
                if (dir == XFRM_POLICY_IN || dir == XFRM_POLICY_FWD) {
                        src = &c->spd.that.host_addr;
                        dst = &c->spd.this.host_addr;
                        set_text_said(n, src, sa.spi, proto);
                } else {
                        src = &c->spd.this.host_addr;
                        dst = &c->spd.that.host_addr;
                        set_text_said(n, dst, sa.spi, proto);
                        }
                }
        }

I'm a little surprised and puzzled that the second args to
set_text_said calls go
        dst
        src
        src
        dst
My intuition says that these should each be "dst".

Even if that's not correct, it feels as if the either the first pair
or the second pair is reversed.

Note: the text_said only appears in logging so being wrong has no
serious consequences.

Observations:

If !(dir == XFRM_POLICY_IN || dir == XFRM_POLICY_FWD)
then dir == XFRM_POLICY_OUT.

Kernel IPSec SA's are unidirectional.
The SAID includes the destination address of packets carried by the SA.
I think that in each case, the destination address is "dst".

There are currently three calls to this function, all within one 
statement of the function netlink_migrate_sa:
        return
                create_xfrm_migrate_sa(st, XFRM_POLICY_OUT, &sa, mig_said) &&
                migrate_xfrm_sa(&sa, st->st_logger) &&

                create_xfrm_migrate_sa(st, XFRM_POLICY_IN, &sa, mig_said) &&
                migrate_xfrm_sa(&sa, st->st_logger) &&

                create_xfrm_migrate_sa(st, XFRM_POLICY_FWD, &sa, mig_said) &&
                migrate_xfrm_sa(&sa, st->st_logger);


dir is the second argument of create_xfrm_migrate_sa.  Clearly it can
only have one of three values: XFRM_POLICY_OUT, XFRM_POLICY_IN,
XFRM_POLICY_FWD.  So the test of dir could be simplified to
        dir != XFRM_POLICY_OUT

Each IPSec SA is unidirectional.  That's why they come in pairs, one
for each direction.

The SAID is a triple: destination IP, SPI, protocol (eg. AH or ESP).

The test dir == XFRM_POLICY_IN || dir == XFRM_POLICY_FWD must be
telling us whether we're dealing with an inbound SA.  If it's true, it
must be inbound.
_______________________________________________
Swan-dev mailing list
Swan-dev@lists.libreswan.org
https://lists.libreswan.org/mailman/listinfo/swan-dev

Reply via email to