[Swan-dev] considertation of marks in route_owner()

[Paul Wouters]

On Wed, 15 May 2019, D. Hugh Redelmeier wrote:

> I think that this means that two conns are considered not to clash if
> they differ in their marks.

Yes. Although as you note below, it is a little tricky. Marks are used
in one direction, meaning it only matches an inbound or outbound IPsec
SA. Two conns with the same in mark but different out mark, would clash
with each other but would not be "identical" from a route_owner's point
of view.

> The code that implements this is:
>
> 	if ( (c->sa_marks.in.val & c->sa_marks.in.mask) != (d->sa_marks.in.val & d->sa_marks.in.mask) &&
> 	     (c->sa_marks.out.val & c->sa_marks.out.mask) != (d->sa_marks.out.val & d->sa_marks.out.mask))
> 		continue;
>
> the "continue" means that we consider the two to be different, and we
> can stop considering this connection (*d).
>
> Why is the && correct?  Should they not be considered distinct if
> EITHER in or out marks differ?
>
> If my suspicion is right, the code should be corrected to:
> 	if ( (c->sa_marks.in.val & c->sa_marks.in.mask) != (d->sa_marks.in.val & d->sa_marks.in.mask) ||
> 	     (c->sa_marks.out.val & c->sa_marks.out.mask) != (d->sa_marks.out.val & d->sa_marks.out.mask))
> 		continue;

I think it would be more correct, but see above. It would probably still
cuase problems, although that might not be this function's problem.

> ================
>
> The following code would be more succinct, and I find it easier to
> understand, but perhaps others would not.  What do you think?
>
> 	if ( (c->sa_marks.in.val ^ d->sa_marks.in.val) & d->sa_marks.in.mask) |
> 	     (c->sa_marks.out.val ^ d->sa_marks.out.val) & d->sa_marks.out.mask) )
> 		continue;

I would not find this clear at all :)

Paul