On 11/20/23 11:42, Vladislav Odintsov wrote:
> Hi Ilya,
>
>> On 15 Nov 2023, at 21:51, Ilya Maximets <i.maxim...@ovn.org> wrote:
>>
>> On 11/15/23 14:13, Vladislav Odintsov wrote:
>>> Hi Ilya,
>>>
>>>> On 13 Nov 2023, at 22:25, Ilya Maximets <i.maxim...@ovn.org> wrote:
>>>>
>>>> On 11/13/23 13:13, Eelco Chaudron wrote:
>>>>>
>>>>>
>>>>> On 13 Nov 2023, at 12:43, Vladislav Odintsov wrote:
>>>>>
>>>>>>> On 13 Nov 2023, at 14:17, Eelco Chaudron <echau...@redhat.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 8 Nov 2023, at 14:39, Vladislav Odintsov wrote:
>>>>>>>
>>>>>>>> Hi Ilya, Eelco,
>>>>>>>>
>>>>>>>> I’ve tried this patch against 3.1 and latest master branch. There are
>>>>>>>> no warnings anymore,
>>>>>>>> but it seems that in my installation it has broken offload capability.
>>>>>>>
>>>>>>> Yes, this is expected, this specific flow can not be offloaded with TC
>>>>>>> and therefore will be processed by the kernel datapath.
>>>>>>
>>>>>> But why did it work before the patch? The traffic was offloaded to it
>>>>>> was flowing correctly.
>>>>>
>>>>> It seemed to work, but your rule had an action to set the source port to
>>>>> 59507, however, this is not happening with tc.
>>>>>
>>>>>
>>>>> actions:set(tunnel(tun_id=0xff0011,src=10.1.0.109,dst=10.1.1.18,ttl=64,tp_src=59507,tp_dst=6081,geneve({class=0x102,type=0x80,len=4,0x18000b}),flags(df|csum|key))),4
>>>>>
>>>>> If you still want to offload this flow, you should not configure the
>>>>> tp_src for this action, and it will be offloaded.
>>>>>
>>>>> Ilya, is this done by OVN? If so, it might need a change there also.
>>>>
>>>> I don't think there is a direct way to force the tp_src into the action.
>>>> OVS is making decision based on the set of flows it has, but I'm not
>>>> sure why exactly.
>>>>
>>>> Vladislav, could you try running ofproto/trace for the packet of interest
>>>> on your setup? This may shed some light on why exactly OVS wants this
>>>> field to be part of the action.
>>>
>>> There were no DP flows with tp_src in action in ovs-appctl dpctl/dump-flows
>>> output, so I grab such flow (with tp_src set in action) from ovs-vswitchd
>>> logs
>>> with enabled dpif_netlink dbg loglevel:
>>>
>>> 2023-11-15T12:52:36.279Z|00056|dpif_netlink(handler3)|DBG|added flow
>>> 2023-11-15T12:52:36.279Z|00057|dpif_netlink(handler3)|DBG|system@ovs-system:
>>> put[create] ufid:23548c16-67c6-47af-a710-2d80cab0a361
>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),tunnel(tun_id=0x6,src=10.1.0.103,dst=10.1.0.109,ttl=64/0,tp_src=3275/0,tp_dst=6081/0,geneve({class=0x102,type=0x80,len=4,0x40003}),flags(-df+csum+key)),in_port(5),skb_mark(0/0),ct_state(0/0x2f),ct_zone(0/0),ct_mark(0/0),ct_label(0/0x3),eth(src=00:00:c9:99:bd:0e,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=169.254.100.1/0.0.0.0,tip=169.254.100.3,op=1,sha=00:00:c9:99:bd:0e/00:00:00:00:00:00,tha=00:00:00:00:00:00/00:00:00:00:00:00),
>>>
>>> actions:set(tunnel(tun_id=0xff0002,src=10.1.0.109,dst=10.1.1.18,ttl=64,tp_src=3275,tp_dst=6081,geneve({class=0x102,type=0x80,len=4,0x55008e}),flags(df|csum|key))),5
>>>
>>> and tracing:
>>>
>>> # ovs-appctl ofproto/trace
>>> 'recirc_id(0),dp_hash(0/0),skb_priority(0/0),tunnel(tun_id=0x6,src=10.1.0.103,dst=10.1.0.109,ttl=64/0,tp_src=3275/0,tp_dst=6081/0,geneve({class=0x102,type=0x80,len=4,0x40003}),flags(-df+csum+key)),in_port(5),skb_mark(0/0),ct_state(0/0x2f),ct_zone(0/0),ct_mark(0/0),ct_label(0/0x3),eth(src=00:00:c9:99:bd:0e,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=169.254.100.1/0.0.0.0,tip=169.254.100.3,op=1,sha=00:00:c9:99:bd:0e/00:00:00:00:00:00,tha=00:00:00:00:00:00/00:00:00:00:00:00)'
>>> Flow:
>>> arp,tun_id=0x6,tun_src=10.1.0.103,tun_dst=10.1.0.109,tun_ipv6_src=::,tun_ipv6_dst=::,tun_gbp_id=0,tun_gbp_flags=0,tun_tos=0,tun_ttl=64,tun_erspan_ver=0,gtpu_flags=0,gtpu_msgtype=0,tun_flags=csum|key,tun_metadata0=0x40003,in_port=17,vlan_tci=0x0000,dl_src=00:00:c9:99:bd:0e,dl_dst=ff:ff:ff:ff:ff:ff,arp_spa=169.254.100.1,arp_tpa=169.254.100.3,arp_op=1,arp_sha=00:00:c9:99:bd:0e,arp_tha=00:00:00:00:00:00
>>>
>>> bridge("br-int")
>>> ----------------
>>> 0. in_port=17, priority 100
>>> move:NXM_NX_TUN_ID[0..23]->OXM_OF_METADATA[0..23]
>>> -> OXM_OF_METADATA[0..23] is now 0x6
>>> move:NXM_NX_TUN_METADATA0[16..30]->NXM_NX_REG14[0..14]
>>> -> NXM_NX_REG14[0..14] is now 0x4
>>> move:NXM_NX_TUN_METADATA0[0..15]->NXM_NX_REG15[0..15]
>>> -> NXM_NX_REG15[0..15] is now 0x3
>>> resubmit(,38)
>>> 38. reg15=0x3,metadata=0x6, priority 100, cookie 0xdb490628
>>> set_field:0x2->reg15
>>> set_field:0x2->reg11
>>> set_field:0x6->reg12
>>> resubmit(,39)
>>> 39. priority 0
>>> set_field:0->reg0
>>> set_field:0->reg1
>>> set_field:0->reg2
>>> set_field:0->reg3
>>> set_field:0->reg4
>>> set_field:0->reg5
>>> set_field:0->reg6
>>> set_field:0->reg7
>>> set_field:0->reg8
>>> set_field:0->reg9
>>> resubmit(,40)
>>> 40. metadata=0x6, priority 0, cookie 0xc3547f56
>>> set_field:0/0x10->xreg4
>>> resubmit(,41)
>>> 41. metadata=0x6, priority 0, cookie 0x64834c48
>>> resubmit(,42)
>>> 42. metadata=0x6, priority 0, cookie 0xc1a1d8a2
>>> resubmit(,43)
>>> 43. metadata=0x6, priority 0, cookie 0x20edcb50
>>> resubmit(,44)
>>> 44. metadata=0x6, priority 0, cookie 0xb22206a8
>>> resubmit(,45)
>>> 45. metadata=0x6, priority 0, cookie 0xe07a9d12
>>> resubmit(,46)
>>> 46. reg15=0x2,metadata=0x6, priority 100, cookie 0x1261fd96
>>> resubmit(,64)
>>> 64. priority 0
>>> resubmit(,65)
>>> 65. reg15=0x2,metadata=0x6, priority 100, cookie 0x903b6c1d
>>>
>>> clone(ct_clear,set_field:0->reg11,set_field:0->reg12,set_field:0->reg13,set_field:0xc->reg11,set_field:0x9->reg12,set_field:0xff0002->metadata,set_field:0x55->reg14,set_field:0->reg10,set_field:0->reg15,set_field:0->reg0,set_field:0->reg1,set_field:0->reg2,set_field:0->reg3,set_field:0->reg4,set_field:0->reg5,set_field:0->reg6,set_field:0->reg7,set_field:0->reg8,set_field:0->reg9,resubmit(,8))
>>> ct_clear
>>> set_field:0->reg11
>>> set_field:0->reg12
>>> set_field:0->reg13
>>> set_field:0xc->reg11
>>> set_field:0x9->reg12
>>> set_field:0xff0002->metadata
>>> set_field:0x55->reg14
>>> set_field:0->reg10
>>> set_field:0->reg15
>>> set_field:0->reg0
>>> set_field:0->reg1
>>> set_field:0->reg2
>>> set_field:0->reg3
>>> set_field:0->reg4
>>> set_field:0->reg5
>>> set_field:0->reg6
>>> set_field:0->reg7
>>> set_field:0->reg8
>>> set_field:0->reg9
>>> resubmit(,8)
>>> 8. metadata=0xff0002, priority 50, cookie 0x577856d6
>>> set_field:0/0x1000->reg10
>>> resubmit(,73)
>>> 73. No match.
>>> drop
>>> move:NXM_NX_REG10[12]->NXM_NX_XXREG0[111]
>>> -> NXM_NX_XXREG0[111] is now 0
>>> resubmit(,9)
>>> 9. metadata=0xff0002, priority 0, cookie 0x5ccc67cb
>>> resubmit(,10)
>>> 10. metadata=0xff0002, priority 0, cookie 0xfa1655a9
>>> resubmit(,11)
>>> 11. metadata=0xff0002, priority 0, cookie 0x573cb297
>>> resubmit(,12)
>>> 12. metadata=0xff0002, priority 0, cookie 0x9484d696
>>> resubmit(,13)
>>> 13. metadata=0xff0002,dl_dst=01:00:00:00:00:00/01:00:00:00:00:00, priority
>>> 110, cookie 0x318d5cd4
>>> resubmit(,14)
>>> 14. metadata=0xff0002, priority 0, cookie 0x2e0a4e13
>>> resubmit(,15)
>>> 15. metadata=0xff0002, priority 65535, cookie 0x9f411194
>>> resubmit(,16)
>>> 16. metadata=0xff0002, priority 65535, cookie 0x83a5ca2c
>>> resubmit(,17)
>>> 17. metadata=0xff0002, priority 0, cookie 0xe87f9407
>>> resubmit(,18)
>>> 18. metadata=0xff0002, priority 0, cookie 0x75403eb9
>>> resubmit(,19)
>>> 19. metadata=0xff0002, priority 0, cookie 0xc18625c0
>>> resubmit(,20)
>>> 20. metadata=0xff0002, priority 0, cookie 0xb81b324
>>> resubmit(,21)
>>> 21. metadata=0xff0002, priority 0, cookie 0x1d3cdf0d
>>> resubmit(,22)
>>> 22. metadata=0xff0002, priority 0, cookie 0xe7e06c1e
>>> resubmit(,23)
>>> 23. metadata=0xff0002, priority 0, cookie 0x46d2ac7d
>>> resubmit(,24)
>>> 24. metadata=0xff0002, priority 0, cookie 0x3b63f9de
>>> resubmit(,25)
>>> 25. metadata=0xff0002, priority 0, cookie 0x3a0b4c7c
>>> resubmit(,26)
>>> 26. metadata=0xff0002, priority 0, cookie 0x4868c582
>>> resubmit(,27)
>>> 27. metadata=0xff0002, priority 0, cookie 0xe247626
>>> resubmit(,28)
>>> 28. metadata=0xff0002, priority 0, cookie 0xd0f0fe80
>>> resubmit(,29)
>>> 29. metadata=0xff0002, priority 0, cookie 0xa1decea3
>>> resubmit(,30)
>>> 30. metadata=0xff0002, priority 0, cookie 0x72a20311
>>> resubmit(,31)
>>> 31. arp,metadata=0xff0002,dl_src=00:00:c9:99:bd:0e,arp_op=1, priority 75,
>>> cookie 0x1baa2454
>>> set_field:0x8004->reg15
>>> resubmit(,37)
>>> 37. reg15=0x8004,metadata=0xff0002, priority 100, cookie 0xfef4a15e
>>> set_field:0x8e->reg15
>>> set_field:0xff0002/0xffffff->tun_id
>>> set_field:0x8e->tun_metadata0
>>> move:NXM_NX_REG14[0..14]->NXM_NX_TUN_METADATA0[16..30]
>>> -> NXM_NX_TUN_METADATA0[16..30] is now 0x55
>>> output:9
>>> -> output to kernel tunnel
>>> set_field:0x8004->reg15
>>>
>>> Final flow:
>>> arp,reg11=0x2,reg12=0x6,reg14=0x4,reg15=0x2,tun_id=0x6,tun_src=10.1.0.103,tun_dst=10.1.0.109,tun_ipv6_src=::,tun_ipv6_dst=::,tun_gbp_id=0,tun_gbp_flags=0,tun_tos=0,tun_ttl=64,tun_erspan_ver=0,gtpu_flags=0,gtpu_msgtype=0,tun_flags=csum|key,tun_metadata0=0x40003,metadata=0x6,in_port=17,vlan_tci=0x0000,dl_src=00:00:c9:99:bd:0e,dl_dst=ff:ff:ff:ff:ff:ff,arp_spa=169.254.100.1,arp_tpa=169.254.100.3,arp_op=1,arp_sha=00:00:c9:99:bd:0e,arp_tha=00:00:00:00:00:00
>>> Megaflow:
>>> recirc_id=0,ct_state=-new-est-rel-rpl-trk,ct_label=0/0x3,eth,arp,tun_id=0x6,tun_src=10.1.0.103,tun_dst=10.1.0.109,tun_tos=0,tun_flags=-df+csum+key,tun_metadata0=0x40003,in_port=17,dl_src=00:00:c9:99:bd:0e,dl_dst=ff:ff:ff:ff:ff:ff,arp_tpa=169.254.100.3,arp_op=1
>>> Datapath actions:
>>> set(tunnel(tun_id=0xff0002,src=10.1.0.109,dst=10.1.1.18,ttl=64,tp_src=3275,tp_dst=6081,geneve({class=0x102,type=0x80,len=4,0x55008e}),flags(df|csum|key))),5
>>>
>>> The traffic is an OVN interconnection usecase, where there is a transit
>>> switch
>>> and two LRs are connected to it on different Availability Zones. LR on AZ1
>>> tries to resolve ARP for LR in another AZ.
>>>
>>> I’d like to add here, that I see if there is a tp_src field in match
>>> portion,
>>> there will be tp_src in action part. And vice versa.
>>
>> OK, I see what is going on here. The traffic is received from one tunnel
>> and is being sent to a different one. It's technically not necessary to
>> set the tp_src in this particular case, so we may be abe to improve this
>> and allow offloading. But some very careful checks needed, since the
>> filled value may come not only from the previous tunnel metadata, but also
>> can be set through other actions, in which case we can't simply drop it.
>> Needs some investigation.
>>
>> BTW, is this only for ARP traffic, or some heavy datapath traffic like
>> TCP/UDP is also flowing through this path and you actually care for it
>> to be offloaded?
>
> ICMP/tcp also not offloaded with patch applied, but offload is working on
> current master branch.
Ack. I posted a v2 adding a test case for the issue:
https://patchwork.ozlabs.org/project/openvswitch/patch/20231201230836.3093792-1-i.maxim...@ovn.org/
I also posted a potential fix that would enable offloading:
https://patchwork.ozlabs.org/project/openvswitch/patch/20231201210523.3085560-1-i.maxim...@ovn.org/
>
>>
>>>
>>>>
>>>> But, as Eelco said, as long as this field is part of the action, we
>>>> can't allow it to be offloaded, just because TC doesn't support it.
>>>
>>> Do I understand correctly, that this patch introduced a kind of regression
>>> in
>>> my case because earlier kernel ignored tp_src, which was set by OVS and the
>>> tc
>>> rule was installed and traffic was HW-offloaded and worked fine.
>>> And now OVS sees it tries to offload a flow with tp_src set and it decides
>>> such
>>> flow is not allowed to be offloaded?
>>
>> Yes. Currently the vaue is just getting ignored, because there is no
>> way to deliver it to TC and offloading kind of "works". With the fix
>> applied, OVS doesn't allow that to happen.
>>
>>>
>>>>
>>>> Best regards, Ilya Maximets.
>>>> _______________________________________________
>>>> dev mailing list
>>>> d...@openvswitch.org <mailto:d...@openvswitch.org>
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>>>
>>>
>>> Regards,
>>> Vladislav Odintsov
>>>
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org <mailto:d...@openvswitch.org>
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>
>
> Regards,
> Vladislav Odintsov
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
