On 31/01/2019 15:32, Roi Dayan wrote:
>
> On 31/01/2019 11:58, Simon Horman wrote:
>> On Mon, Jan 21, 2019 at 05:32:37PM +0200, Adi Nissim wrote:
>>> Currently the TC tunnel_key action is always
>>> initialized with the given tunnel id value. However,
>>> some tunneling protocols define the tunnel id as an optional field.
>>>
>>> This patch initializes the id field of tunnel_key:set and tunnel_key:unset
>>> only if a value is provided.
>>>
>>> In the case that a tunnel key value is not provided by the user
>>> the key flag will not be set.
>>>
>>> Signed-off-by: Adi Nissim <[email protected]>
>>> Acked-by: Paul Blakey <[email protected]>
>>> ---
>>> v1->v2: check if mask.tunnel.id == OVS_BE64_MAX
>>> so we won't do match in the case of a partial mask.
>> I am still a bit concerned about the partial mask case.
>> It looks like the code will now silently not offload such matches.
>>
>> I think that a partial mask should either be offloaded or
>> offload of the entire flow should be rejected.
> thanks. you are right. I didn't notice it. partial masks should be rejected
> to fallback to ovs dp instead of ignoring the mask.
>
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;
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);
+ }
let me know what you think.
Thanks,
Roi
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
