> 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.
>
>
>> 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 <echau...@redhat.com> 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:
>>>> 00000010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
>>>> 00000020 c0 5b 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
>>>> 00000050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>> 00000060 01 02 80 01 00 18 00 0b-00 00 00 00 00 00 00 00
>>>> ...
>>>> - Received Action:
>>>> 00000010 01 00 00 00 00 00 00 00-00 00 00 00 00 ff 00 11
>>>> 00000020 00 00 17 c1 00 40 00 00-0a 01 00 6d 0a 01 01 12
>>>> 00000050 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>>> 00000060 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 <odiv...@gmail.com>
>>>> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
>>>
>>> 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 <echau...@redhat.com <mailto:echau...@redhat.com>
>>> <mailto:echau...@redhat.com>>
>>>
>>> _______________________________________________
>>> dev mailing list
>>> d...@openvswitch.org <mailto:d...@openvswitch.org>
>>> <mailto:d...@openvswitch.org>
>>> 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
Regards,
Vladislav Odintsov
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
