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

[Eyal Birger <eyal.birger@gmail.com>]

Hi Nathan,

On Fri, 29 Jun 2018 15:07:10 -0700
Nathan Harold <nharold@google.com> wrote:

> Allow UPDSA to change "set mark" to permit
> policy separation of packet routing decisions from
> SA keying in systems that use mark-based routing.
> 
> The set 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 set mark is only
> updated if sent with a non-zero value.
> 
> The per-SA lock and the xfrm_state_lock are taken in
> that order to avoid a deadlock with
> xfrm_timer_handler(), which also takes the locks in
> that order.
> 
> Signed-off-by: Nathan Harold <nharold@google.com>
> Change-Id: Ia05c6733a94c1901cd1e54eb7c7e237704678d71
> ---
>  net/xfrm/xfrm_state.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index e04a510ec992..c9ffcdfa89f6 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -1562,6 +1562,15 @@ int xfrm_state_update(struct xfrm_state *x)
>  		if (x1->curlft.use_time)
>  			xfrm_state_check_expire(x1);
>  
> +		if (x->props.smark.m || x->props.smark.v) {
> +			spin_lock_bh(&net->xfrm.xfrm_state_lock);
> +
> +			x1->props.smark = x->props.smark;
> +
> +			__xfrm_state_bump_genids(x1);

So I'm trying to wrap my head around this genid thing :)

If x1 points to a state previously found using __xfrm_state_locate(x),
won't __xfrm_state_bump_genids(x1) be equivalent to x1->genid++ in
this case?

Is it possible that other states will match all of x1 parameters?

Also, any idea why this isn't needed for other changes in the state?

Thanks!
Eyal.