On 04/02/2019 15:20, Roi Dayan wrote:
>>>>> Hi Simon,
>>>>>
>>>>> I did some checks and think the correct fix is to offload exact match.
>>>>> if key is partial we can ignore the mask and offload exact match and
>>>>> it will be correct as we do more strict matching.
>>>>>
>>>>> it is also documented that the kernel datapath is doing the same
>>>>> (from datapath.rst)
>>>>>
>>>>> "The kernel can ignore the mask attribute, installing an exact
>>>>> match flow"
>>>>>
>>>>> So I think the first patch V0 is actually correct as we
>>>>> check the tunnel key flag exists and offload exact match if
>>>>> there was any mask or offload without a key if the mask is 0
>>>>> or no key.
>>>>>
>>>>> in netdev-tc-offloads.c
>>>>>
>>>>> + flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ?
>>>>> + tnl_mask->tun_id : 0;
>>>>>
>>>> I think this is fine so long as tun_id is all-ones. Is that always the
>>>> case?
>>>> Should the code check that it is the case? Am I missing the point?
>>>>
>>> it looks like tun_id mask is always set to all-ones.
>>> but even if it won't be in the future, we shouldn't really care here.
>>> tc adds exact match on the tun_id and ignores the tun_id mask.
>>> this is considered ok as the matching is more strict.
>>> if new match is needed with different tun_id then ovs will try to add
>>> another rule for it.
>>> so with tc we could have multiple rules vs 1 rule that support mask.
>>
>> Thanks for looking into this. That sounds find to me but I wonder if we
>> should make
>> this behaviour explicit.
>>
>> /*
>> * Comment describing why the mask is 0 or all-ones
>> */
>> flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? UINT32_MAX :
>> 0;
>>
> I think its nicer like this and symetric currently. here it's generic
> and "use" mask.
> in tc.c when we fill the netlink msg we ignore the mas and also when
> parsing tc dump,
> tun mask is set to FFFF here (tc.c) and not netdev-tc-offloads.c
>
Hi Simon,
any update? i remind i suggested v0 is the correct one.
Thanks,
Roi
>>>>>
>>>>> in tc.c
>>>>>
>>>>> - nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
>>>>> + if (id_mask) {
>>>>> + nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
>>>>> + }
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
