Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
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.
In tcpdump I see packets, which I expect to be offloaded,
so not to appear in tcpdump output, also there is an empty output
of ovs-appctl dpctl/dump-flows type=offloaded.
On ovs without this patch the same host offloads this traffic but with warnings.
Let me know which debug information you need from me.
As a reminder, the test system runs CentOS 8.4 with 6.5.7-1.el8.elrepo.x86_64
elrepo kernel.
SmartNIC is Mellanox Technologies MT2892 Family [ConnectX-6 Dx].
> On 31 Oct 2023, at 11:33, Eelco Chaudron wrote:
>
>
>
> On 30 Oct 2023, at 15:00, Ilya Maximets wrote:
>
>> There is no TCA_TUNNEL_KEY_ENC_SRC_PORT in the kernel, so the offload
>> should not be attempted if OVS_TUNNEL_KEY_ATTR_TP_SRC is requested
>> by OVS. Current code just ignores the attribute in the tunnel(set())
>> action leading to a flow mismatch and potential incorrect datapath
>> behavior:
>>
>> |tc(handler21)|DBG|tc flower compare failed action compare
>> ...
>> Action 0 mismatch:
>> - Expected Action:
>> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
>> 0020 c0 5b 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
>> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
>> ...
>> - Received Action:
>> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
>> 0020 00 00 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
>> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
>> ...
>>
>> In the tc_action dump above we can see the difference on the second
>> line. The action dumped from a kernel is missing 'c0 5b' - source
>> port for a tunnel(set()) action on the second line.
>>
>> Removing the field from the tc_action_encap structure entirely to
>> avoid any potential confusion.
>>
>> Note: In general, the source port number in the tunnel(set()) action
>> is not particularly useful for most tunnels, because they may just
>> ignore the value. Specs for Geneve and VXLAN suggest using a value
>> based on the headers of the inner packet as a source port.
>> And I'm not really sure how to make OVS to generate the action with
>> a source port in it, so the commit doesn't include the test case.
>> In vast majority of scenarios the source port doesn't actually end
>> up in the action itself.
>> Having a mismatch between the userspace and TC leads to constant
>> revalidation of the flow and warnings in the log, and might
>> potentially cause mishandling of the traffic, even though the impact
>> is unclear at the moment.
>>
>> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc
>> interface")
>> Reported-at:
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2023-October/052744.html
>> Reported-by: Vladislav Odintsov
>> Signed-off-by: Ilya Maximets
>
> Thanks Ilya, this change seems correct as the kernel does not seem to support
> setting the source port. I did some additional tests, and found no problems.
>
> Acked-by: Eelco Chaudron mailto:echau...@redhat.com>>
>
> ___
> dev mailing list
> d...@openvswitch.org
> 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
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
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.
> In tcpdump I see packets, which I expect to be offloaded,
> so not to appear in tcpdump output, also there is an empty output
> of ovs-appctl dpctl/dump-flows type=offloaded.
>
> On ovs without this patch the same host offloads this traffic but with
> warnings.
> Let me know which debug information you need from me.
>
> As a reminder, the test system runs CentOS 8.4 with 6.5.7-1.el8.elrepo.x86_64
> elrepo kernel.
> SmartNIC is Mellanox Technologies MT2892 Family [ConnectX-6 Dx].
>
>> On 31 Oct 2023, at 11:33, Eelco Chaudron wrote:
>>
>>
>>
>> On 30 Oct 2023, at 15:00, Ilya Maximets wrote:
>>
>>> There is no TCA_TUNNEL_KEY_ENC_SRC_PORT in the kernel, so the offload
>>> should not be attempted if OVS_TUNNEL_KEY_ATTR_TP_SRC is requested
>>> by OVS. Current code just ignores the attribute in the tunnel(set())
>>> action leading to a flow mismatch and potential incorrect datapath
>>> behavior:
>>>
>>> |tc(handler21)|DBG|tc flower compare failed action compare
>>> ...
>>> Action 0 mismatch:
>>> - Expected Action:
>>> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
>>> 0020 c0 5b 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
>>> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
>>> ...
>>> - Received Action:
>>> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
>>> 0020 00 00 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
>>> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
>>> ...
>>>
>>> In the tc_action dump above we can see the difference on the second
>>> line. The action dumped from a kernel is missing 'c0 5b' - source
>>> port for a tunnel(set()) action on the second line.
>>>
>>> Removing the field from the tc_action_encap structure entirely to
>>> avoid any potential confusion.
>>>
>>> Note: In general, the source port number in the tunnel(set()) action
>>> is not particularly useful for most tunnels, because they may just
>>> ignore the value. Specs for Geneve and VXLAN suggest using a value
>>> based on the headers of the inner packet as a source port.
>>> And I'm not really sure how to make OVS to generate the action with
>>> a source port in it, so the commit doesn't include the test case.
>>> In vast majority of scenarios the source port doesn't actually end
>>> up in the action itself.
>>> Having a mismatch between the userspace and TC leads to constant
>>> revalidation of the flow and warnings in the log, and might
>>> potentially cause mishandling of the traffic, even though the impact
>>> is unclear at the moment.
>>>
>>> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using
>>> tc interface")
>>> Reported-at:
>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2023-October/052744.html
>>> Reported-by: Vladislav Odintsov
>>> Signed-off-by: Ilya Maximets
>>
>> Thanks Ilya, this change seems correct as the kernel does not seem to
>> support setting the source port. I did some additional tests, and found no
>> problems.
>>
>> Acked-by: Eelco Chaudron mailto:echau...@redhat.com>>
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> 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
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
> On 13 Nov 2023, at 14:17, Eelco Chaudron 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.
>
>
>> In tcpdump I see packets, which I expect to be offloaded,
>> so not to appear in tcpdump output, also there is an empty output
>> of ovs-appctl dpctl/dump-flows type=offloaded.
>>
>> On ovs without this patch the same host offloads this traffic but with
>> warnings.
>> Let me know which debug information you need from me.
>>
>> As a reminder, the test system runs CentOS 8.4 with
>> 6.5.7-1.el8.elrepo.x86_64 elrepo kernel.
>> SmartNIC is Mellanox Technologies MT2892 Family [ConnectX-6 Dx].
>>
>>> On 31 Oct 2023, at 11:33, Eelco Chaudron wrote:
>>>
>>>
>>>
>>> On 30 Oct 2023, at 15:00, Ilya Maximets wrote:
>>>
There is no TCA_TUNNEL_KEY_ENC_SRC_PORT in the kernel, so the offload
should not be attempted if OVS_TUNNEL_KEY_ATTR_TP_SRC is requested
by OVS. Current code just ignores the attribute in the tunnel(set())
action leading to a flow mismatch and potential incorrect datapath
behavior:
|tc(handler21)|DBG|tc flower compare failed action compare
...
Action 0 mismatch:
- Expected Action:
0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
0020 c0 5b 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
...
- Received Action:
0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
0020 00 00 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
...
In the tc_action dump above we can see the difference on the second
line. The action dumped from a kernel is missing 'c0 5b' - source
port for a tunnel(set()) action on the second line.
Removing the field from the tc_action_encap structure entirely to
avoid any potential confusion.
Note: In general, the source port number in the tunnel(set()) action
is not particularly useful for most tunnels, because they may just
ignore the value. Specs for Geneve and VXLAN suggest using a value
based on the headers of the inner packet as a source port.
And I'm not really sure how to make OVS to generate the action with
a source port in it, so the commit doesn't include the test case.
In vast majority of scenarios the source port doesn't actually end
up in the action itself.
Having a mismatch between the userspace and TC leads to constant
revalidation of the flow and warnings in the log, and might
potentially cause mishandling of the traffic, even though the impact
is unclear at the moment.
Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using
tc interface")
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2023-October/052744.html
Reported-by: Vladislav Odintsov
Signed-off-by: Ilya Maximets
>>>
>>> Thanks Ilya, this change seems correct as the kernel does not seem to
>>> support setting the source port. I did some additional tests, and found no
>>> problems.
>>>
>>> Acked-by: Eelco Chaudron mailto:echau...@redhat.com>
>>> >
>>>
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>>
>>> 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
Regards,
Vladislav Odintsov
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
On 13 Nov 2023, at 12:43, Vladislav Odintsov wrote:
>> On 13 Nov 2023, at 14:17, Eelco Chaudron 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.
>>> In tcpdump I see packets, which I expect to be offloaded,
>>> so not to appear in tcpdump output, also there is an empty output
>>> of ovs-appctl dpctl/dump-flows type=offloaded.
>>>
>>> On ovs without this patch the same host offloads this traffic but with
>>> warnings.
>>> Let me know which debug information you need from me.
>>>
>>> As a reminder, the test system runs CentOS 8.4 with
>>> 6.5.7-1.el8.elrepo.x86_64 elrepo kernel.
>>> SmartNIC is Mellanox Technologies MT2892 Family [ConnectX-6 Dx].
>>>
On 31 Oct 2023, at 11:33, Eelco Chaudron wrote:
On 30 Oct 2023, at 15:00, Ilya Maximets wrote:
> There is no TCA_TUNNEL_KEY_ENC_SRC_PORT in the kernel, so the offload
> should not be attempted if OVS_TUNNEL_KEY_ATTR_TP_SRC is requested
> by OVS. Current code just ignores the attribute in the tunnel(set())
> action leading to a flow mismatch and potential incorrect datapath
> behavior:
>
> |tc(handler21)|DBG|tc flower compare failed action compare
> ...
> Action 0 mismatch:
> - Expected Action:
> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
> 0020 c0 5b 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
> ...
> - Received Action:
> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
> 0020 00 00 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
> ...
>
> In the tc_action dump above we can see the difference on the second
> line. The action dumped from a kernel is missing 'c0 5b' - source
> port for a tunnel(set()) action on the second line.
>
> Removing the field from the tc_action_encap structure entirely to
> avoid any potential confusion.
>
> Note: In general, the source port number in the tunnel(set()) action
> is not particularly useful for most tunnels, because they may just
> ignore the value. Specs for Geneve and VXLAN suggest using a value
> based on the headers of the inner packet as a source port.
> And I'm not really sure how to make OVS to generate the action with
> a source port in it, so the commit doesn't include the test case.
> In vast majority of scenarios the source port doesn't actually end
> up in the action itself.
> Having a mismatch between the userspace and TC leads to constant
> revalidation of the flow and warnings in the log, and might
> potentially cause mishandling of the traffic, even though the impact
> is unclear at the moment.
>
> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using
> tc interface")
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2023-October/052744.html
> Reported-by: Vladislav Odintsov
> Signed-off-by: Ilya Maximets
Thanks Ilya, this change seems correct as the kernel does not seem to
support setting the source port. I did some additional tests, and found no
problems.
Acked-by: Eelco Chaudron mailto:echau...@redhat.com>
>
___
dev mailing list
d...@openvswitch.org
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
>
>
> Regards,
> Vladislav Odintsov
___
dev mailing list
d...@openvswitch.org
https://mail.op
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
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 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.
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.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
(re-send as for some reason gmail rejected the previous attempt
and it wasn't delivered to Vladislav)
On 11/13/23 20:25, Ilya Maximets 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 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.
>
> 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.
>
> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
On Mon, Oct 30, 2023 at 03:00:29PM +0100, Ilya Maximets wrote: > There is no TCA_TUNNEL_KEY_ENC_SRC_PORT in the kernel, so the offload > should not be attempted if OVS_TUNNEL_KEY_ATTR_TP_SRC is requested > by OVS. Current code just ignores the attribute in the tunnel(set()) > action leading to a flow mismatch and potential incorrect datapath > behavior: Late comment but, yes.. thanks for fixing this. I also have never seen a flow that ended up matching on the tunnel src port, but still, it was an issue. Marcelo ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
Hi Ilya,
> On 13 Nov 2023, at 22:25, Ilya Maximets 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 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=0x,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
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
On 11/15/23 14:13, Vladislav Odintsov wrote:
> Hi Ilya,
>
>> On 13 Nov 2023, at 22:25, Ilya Maximets 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 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=0x,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
>
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
Hi Vladislav,
On Wed, Nov 15, 2023 at 04:13:13PM +0300, Vladislav Odintsov wrote:
...
> 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=0x,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.
In the flow here I see in_port=17 and output to port 5. Can you please
confirm which ports are those? The 2 ports of the CX6, maybe?
Asking because I'm a bit surprised that hairpin traffic with such
encap operations actually works.
Thanks,
Marcelo
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
Hi Marcelo,
PSB.
> On 16 Nov 2023, at 15:02, Marcelo Ricardo Leitner wrote:
>
> Hi Vladislav,
>
> On Wed, Nov 15, 2023 at 04:13:13PM +0300, Vladislav Odintsov wrote:
> ...
>> 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=0x,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.
>
> In the flow here I see in_port=17 and output to port 5. Can you please
> confirm which ports are those? The 2 ports of the CX6, maybe?
> Asking because I'm a bit surprised that hairpin traffic with such
> encap operations actually works.
Port 5 is a geneve tunnel OVS port.
The next flow is working fine on my setup (first packet reaches system and
others don’t; traffic is flowing offloaded):
2023-11-20T10:29:25.220Z|00034|dpif_netlink(handler1)|DBG|system@ovs-system:
put[create] ufid:417d13c0-d5d9-4832-a155-8e83c99c1fe4
recirc_id(0),dp_hash(0/0),skb_priority(0/0),tunnel(tun_id=0x6,src=10.1.0.107,dst=10.1.0.109,ttl=64/0,tp_src=19915/0,tp_dst=6081/0,geneve({class=0x102,type=0x80,len=4,0x50003}),flags(-df+csum+key)),in_port(4),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=00:01:c9:99:bd:0e),eth_type(0x0800),ipv4(src=172.31.32.6/0.0.0.0,dst=172.31.0.6/0.0.0.0,proto=1,tos=0/0x3,ttl=63/0,frag=no),icmp(type=8/0,code=0/0),
actions:set(tunnel(tun_id=0xff0002,src=10.1.0.109,dst=10.1.1.18,ttl=64,tp_src=19915,tp_dst=6081,geneve({class=0x102,type=0x80,len=4,0x55008e}),flags(df|csum|key))),4
(port 4 is genev_sys_6081 here).
>
> Thanks,
> Marcelo
>
> ___
> dev mailing list
> d...@openvswitch.org
> 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
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
Hi Ilya,
> On 15 Nov 2023, at 21:51, Ilya Maximets wrote:
>
> On 11/15/23 14:13, Vladislav Odintsov wrote:
>> Hi Ilya,
>>
>>> On 13 Nov 2023, at 22:25, Ilya Maximets 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 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=0x,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. me
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
On 11/20/23 11:42, Vladislav Odintsov wrote:
> Hi Ilya,
>
>> On 15 Nov 2023, at 21:51, Ilya Maximets wrote:
>>
>> On 11/15/23 14:13, Vladislav Odintsov wrote:
>>> Hi Ilya,
>>>
On 13 Nov 2023, at 22:25, Ilya Maximets 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 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=0x,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
Re: [ovs-dev] [PATCH] netdev-offload-tc: Fix offload of tunnel key tp_src.
On 30 Oct 2023, at 15:00, Ilya Maximets wrote:
> There is no TCA_TUNNEL_KEY_ENC_SRC_PORT in the kernel, so the offload
> should not be attempted if OVS_TUNNEL_KEY_ATTR_TP_SRC is requested
> by OVS. Current code just ignores the attribute in the tunnel(set())
> action leading to a flow mismatch and potential incorrect datapath
> behavior:
>
> |tc(handler21)|DBG|tc flower compare failed action compare
> ...
> Action 0 mismatch:
> - Expected Action:
> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
> 0020 c0 5b 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
> ...
> - Received Action:
> 0010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
> 0020 00 00 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
> 0050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
> ...
>
> In the tc_action dump above we can see the difference on the second
> line. The action dumped from a kernel is missing 'c0 5b' - source
> port for a tunnel(set()) action on the second line.
>
> Removing the field from the tc_action_encap structure entirely to
> avoid any potential confusion.
>
> Note: In general, the source port number in the tunnel(set()) action
> is not particularly useful for most tunnels, because they may just
> ignore the value. Specs for Geneve and VXLAN suggest using a value
> based on the headers of the inner packet as a source port.
> And I'm not really sure how to make OVS to generate the action with
> a source port in it, so the commit doesn't include the test case.
> In vast majority of scenarios the source port doesn't actually end
> up in the action itself.
> Having a mismatch between the userspace and TC leads to constant
> revalidation of the flow and warnings in the log, and might
> potentially cause mishandling of the traffic, even though the impact
> is unclear at the moment.
>
> Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc
> interface")
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2023-October/052744.html
> Reported-by: Vladislav Odintsov
> Signed-off-by: Ilya Maximets
Thanks Ilya, this change seems correct as the kernel does not seem to support
setting the source port. I did some additional tests, and found no problems.
Acked-by: Eelco Chaudron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
