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

D. Hugh Redelmeier hugh at mimosa.com
Wed May 15 16:56:02 UTC 2019


In connections.c: route_owner() there is a comment:

	/*
	 * allow policies different by mark/mask
	 */

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

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;

================

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 like the space after the initial "(" in complex nested
parenthisized expressions; there should be a corresponding space
before the matching ")".

This may violate "kernel style" but it slightly helps the reader, and
I think that makes it worthwhile.


More information about the Swan-dev mailing list