On 2020/5/22 13:49, Xin Long wrote: > On Fri, May 22, 2020 at 9:45 AM Yuehaibing <yuehaib...@huawei.com> wrote: >> >> On 2020/5/21 14:49, Xin Long wrote: >>> On Tue, May 19, 2020 at 4:53 PM Steffen Klassert >>> <steffen.klass...@secunet.com> wrote: >>>> >>>> On Fri, May 15, 2020 at 04:39:57PM +0800, Yuehaibing wrote: >>>>> >>>>> Friendly ping... >>>>> >>>>> Any plan for this issue? >>>> >>>> There was still no consensus between you and Xin on how >>>> to fix this issue. Once this happens, I consider applying >>>> a fix. >>>> >>> Sorry, Yuehaibing, I can't really accept to do: (A->mark.m & A->mark.v) >>> I'm thinking to change to: >>> >>> static bool xfrm_policy_mark_match(struct xfrm_policy *policy, >>> struct xfrm_policy *pol) >>> { >>> - u32 mark = policy->mark.v & policy->mark.m; >>> - >>> - if (policy->mark.v == pol->mark.v && policy->mark.m == pol->mark.m) >>> - return true; >>> - >>> - if ((mark & pol->mark.m) == pol->mark.v && >>> - policy->priority == pol->priority) >>> + if (policy->mark.v == pol->mark.v && >>> + (policy->mark.m == pol->mark.m || >>> + policy->priority == pol->priority)) >>> return true; >>> >>> return false; >>> >>> which means we consider (the same value and mask) or >>> (the same value and priority) as the same one. This will >>> cover both problems. >> >> policy A (mark.v = 0x1011, mark.m = 0x1011, priority = 1) >> policy B (mark.v = 0x1001, mark.m = 0x1001, priority = 1) > I'd think these are 2 different policies. > >> >> when fl->flowi_mark == 0x12341011, in xfrm_policy_match() do check like >> this: >> >> (fl->flowi_mark & pol->mark.m) != pol->mark.v >> >> 0x12341011 & 0x1011 == 0x00001011 >> 0x12341011 & 0x1001 == 0x00001001 >> >> This also match different policy depends on the order of policy inserting. > Yes, this may happen when a user adds 2 policies like that. > But I think this's a problem that the user doesn't configure it well, > 'priority' should be set. > and this can not be avoided, also such as: > > policy A (mark.v = 0xff00, mark.m = 0x1000, priority = 1) > policy B (mark.v = 0x00ff, mark.m = 0x0011, priority = 1) > > try with 0x12341011 > > So just be it, let users decide.
Ok, this make sense. > > . >