On 31 Jan 2022, at 14:58, Roi Dayan wrote:
> On 2021-12-21 10:55 AM, Roi Dayan wrote:
>>
>>
>> On 2021-12-12 9:45 AM, Roi Dayan wrote:
>>>
>>>
>>> On 2021-12-02 2:38 PM, Roi Dayan wrote:
>>>> A datapath flow generated for traffic from vxlan port to another vxlan port
>>>> looks like this:
>>>>
>>>> tunnel(tun_id=0x65,src=10.10.11.3,dst=10.10.11.2,ttl=0/0,tp_dst=4789,flags(+key)),...,in_port(vxlan_sys_4789),...,
>>>>
>>>> actions:set(tunnel(tun_id=0x66,src=10.10.12.2,dst=10.10.12.3,tp_dst=4789,flags(key))),vxlan_sys_4789
>>>>
>>>> The generated TC rule with explicit tunnel key unset action added after
>>>> tunnel key set action, which is wrong.
>>>>
>>>> filter protocol ip pref 7 flower chain 0 handle 0x1
>>>> dst_mac fa:16:3e:2a:4e:23
>>>> eth_type ipv4
>>>> ip_tos 0x0/3
>>>> enc_dst_ip 10.10.11.2
>>>> enc_src_ip 10.10.11.3
>>>> enc_key_id 101
>>>> enc_dst_port 4789
>>>> ip_flags nofrag
>>>> not_in_hw
>>>> action order 1: tunnel_key set
>>>> src_ip 10.10.12.2
>>>> dst_ip 10.10.12.3
>>>> key_id 102
>>>> dst_port 4789
>>>> nocsum pipe
>>>> index 1 ref 1 bind 1 installed 568 sec used 0 sec
>>>> Action statistics:
>>>> Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
>>>> backlog 0b 0p requeues 0
>>>>
>>>> action order 2: tunnel_key unset pipe
>>>> index 2 ref 1 bind 1 installed 568 sec used 0 sec
>>>> Action statistics:
>>>> Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
>>>> backlog 0b 0p requeues 0
>>>>
>>>> action order 3: mirred (Egress Redirect to device vxlan_sys_4789)
>>>> stolen
>>>> index 1 ref 1 bind 1 installed 568 sec used 0 sec
>>>> Action statistics:
>>>> Sent 46620 bytes 555 pkt (dropped 0, overlimits 0 requeues 0)
>>>> backlog 0b 0p requeues 0
>>>> cookie e0c82bfd504b701428b00db6b08db3b2
>>>>
>>>> Fix it by also adding the the tunnel key unset action before the tunnel
>>>> key set action and not only before output port.
>>>>
>>>> Fixes: 7c53bd7839d8 ("tc: Move tunnel_key unset action before output
>>>> ports")
>>>> Signed-off-by: Roi Dayan <[email protected]>
>>>> Reviewed-by: Paul Blakey <[email protected]>
>>>> ---
>>>>
>>>> Notes:
>>>> v2
>>>> - fix a mistake in the reviewed-by tag email domain.
>>>>
>>>> lib/tc.c | 21 +++++++++++++++++----
>>>> 1 file changed, 17 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/lib/tc.c b/lib/tc.c
>>>> index 38a1dfc0ebc8..adb2d3182ad4 100644
>>>> --- a/lib/tc.c
>>>> +++ b/lib/tc.c
>>>> @@ -2545,6 +2545,17 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf
>>>> *request,
>>>> return 0;
>>>> }
>>>> +static void
>>>> +nl_msg_put_flower_acts_release(struct ofpbuf *request, uint16_t act_index)
>>>> +{
>>>> + size_t act_offset;
>>>> +
>>>> + act_offset = nl_msg_start_nested(request, act_index);
>>>> + nl_msg_put_act_tunnel_key_release(request);
>>>> + nl_msg_put_act_flags(request);
>>>> + nl_msg_end_nested(request, act_offset);
>>>> +}
>>>> +
>>>> static int
>>>> nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
>>>> {
>>>> @@ -2579,6 +2590,11 @@ nl_msg_put_flower_acts(struct ofpbuf *request,
>>>> struct tc_flower *flower)
>>>> }
>>>> break;
>>>> case TC_ACT_ENCAP: {
>>>> + if (!released && flower->tunnel) {
>>>> + nl_msg_put_flower_acts_release(request, act_index++);
>>>> + released = true;
>>>> + }
>>>> +
>>>> act_offset = nl_msg_start_nested(request, act_index++);
>>>> nl_msg_put_act_tunnel_key_set(request,
>>>> action->encap.id_present,
>>>> action->encap.id,
>>>> @@ -2636,10 +2652,7 @@ nl_msg_put_flower_acts(struct ofpbuf *request,
>>>> struct tc_flower *flower)
>>>> break;
>>>> case TC_ACT_OUTPUT: {
>>>> if (!released && flower->tunnel) {
>>>> - act_offset = nl_msg_start_nested(request,
>>>> act_index++);
>>>> - nl_msg_put_act_tunnel_key_release(request);
>>>> - nl_msg_put_act_flags(request);
>>>> - nl_msg_end_nested(request, act_offset);
>>>> + nl_msg_put_flower_acts_release(request, act_index++);
>>>> released = true;
>>>> }
>>>
>>> hi,
>>>
>>> are there any comments on this small fix?
>>>
>>> thanks,
>>> Roi
>>
>>
>> Hi Ilya, Simon,
>>
>> Sorry for nagging. just pinging again on this very small offload fix.
>>
>> Thanks,
>> Roi
>
> Hi Eelco,
>
> Maybe you have time to review this ? :) as you are also familiar with
> this area probably.
I’ll add it to my review todo for this week!
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
