Re: [PATCH ipsec-next] xfrm: Allow Output Mark to be Updated Using UPDSA

[Nathan Harold <nharold@google.com>]

That makes sense to me; the restriction about which you inquire is a
practical one rather than a philosophical one, which I will be happy
to see lifted.

With the new set_mark, a non-zero mask will indicate that the caller
has a set an "explicit" zero mark, which sidesteps the
currently-ambiguous situation; the logic can then become "if (set_mark
|| set_mark_mask) { // update mark and update mask}". There is a
question of the behavior for a caller who sets a set_mark and
set_mark_mask, then subsequently calls UPDSA with only a mark
(omitting the mask, or with explicit set_mask == 0). I think it's fair
and appropriate the mask be re-set to 0xFFFFFFFF (to avoid the
special-case of (if new_set_mask == 0 && set_mask != 0xFFFFFFFF). Of
course, this means that the inability to return to zero limitation
that I currently mention as being on the output_mark would transfer
under that proposal to the set_mark_mask. All of this is fix-able by
having the update take into account the presence or absence of the
XFRMAs sent rather than just looking at a built xfrm_state, but I'm
couldn't fathom any use cases for reverting the mark scheme back to an
"unused" state while the SA remains ACTIVE, so I think simpler is
better (same reasoning applied to the current change).

-Nathan

On Wed, May 9, 2018 at 10:44 PM, Eyal Birger <eyal.birger@gmail.com> wrote:
> Hi Nathan,
>
> On Wed,  9 May 2018 13:46:26 -0700
> Nathan Harold <nharold@google.com> wrote:
>
>> Allow UPDSA to change output_mark to permit
>> policy separation of packet routing decisions from
>> SA keying in systems that use mark-based routing.
>>
>> In the output_mark, used as a routing and firewall
>> mark for outbound packets, is made update-able which
>> allows routing decisions to be handled independently
>> of keying/SA creation. To maintain consistency with
>> other optional attributes, the output mark is only
>> updated if sent with a non-zero value. Once set, the
>> output mark may not be reset to zero, which ensures
>> that updating the SA does not require the mark to
>> be re-sent to avoid the value being clobbered.
>
> There is an attempt to extend the 'output_mark' to support the input
> direction and masking.
>
> In the proposed implementation, output_mark is converted to type 'struct
> xfrm_mark' where the semantics are as follows:
>
> - If mark is given by XFRMA_OUTPUT_MARK (renamed to XFRMA_SET_MARK)
>   then a new XFRMA_SET_MARK_MASK attribute is consulted to set the mask
>   value
> - if no XFRMA_SET_MARK_MASK attribute is provided, the mask is set to
>   0xffffffff
>
> Therefore, if the mask value is 0, we can regard the mark as 'not
> given'.
>
> My question is, in the context of this patch, it seems that the
> "Once set, the output mark may not be reset to zero" restriction may be
> lifted in favor of updating the mark only if the new mask is non zero.
>
> Does this make sense to you?
> Eyal