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.bir...@gmail.com> wrote: > Hi Nathan, > > On Wed, 9 May 2018 13:46:26 -0700 > Nathan Harold <nhar...@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