On 2 Dec 2024, at 13:39, Ilya Maximets wrote:
> On 12/2/24 13:36, Ilya Maximets wrote:
>> On 12/2/24 13:30, Ilya Maximets wrote:
>>> On 12/2/24 13:26, Eelco Chaudron wrote:
>>>>
>>>>
>>>> On 2 Dec 2024, at 13:18, Ilya Maximets wrote:
>>>>
>>>>> On 12/2/24 13:02, Eelco Chaudron wrote:
>>>>>>
>>>>>>
>>>>>> On 2 Dec 2024, at 12:49, Ilya Maximets wrote:
>>>>>>
>>>>>>> On 12/2/24 12:43, Eelco Chaudron wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2 Dec 2024, at 12:34, Ilya Maximets wrote:
>>>>>>>>
>>>>>>>>> On 11/29/24 15:45, Eelco Chaudron wrote:
>>>>>>>
>>>>>>> <snip>
>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi Ales,
>>>>>>>>>>
>>>>>>>>>> It took me a bit longer to figure out what was going on, but I found
>>>>>>>>>> the issue. Your test case creates three DP flows. The first two
>>>>>>>>>> flows uses the dp_hash()/hash() match/action, which is not
>>>>>>>>>> offloadable, while the third flow is simple and offloadable.
>>>>>>>>>>
>>>>>>>>>> What happens next is that the first two flows are processed in the
>>>>>>>>>> ovs kernel module. However, as the third recirculated flow is
>>>>>>>>>> applied in TC, it still does not exist in the kernel DP. As a
>>>>>>>>>> result, the packet is sent for upcall handling. This process repeats
>>>>>>>>>> for every packet as the flow keeps being installed in TC.
>>>>>>>>>>
>>>>>>>>>> - [ovs]
>>>>>>>>>> recirc_id(0),in_port(2),eth(),eth_type(0x0800),ipv4(dst=10.0.0.2,frag=no),
>>>>>>>>>> packets:9, bytes:882, used:0.125s, actions:hash(l4(0)),recirc(0x3)
>>>>>>>>>> - [ovs]
>>>>>>>>>> recirc_id(0x3),dp_hash(0xa/0xf),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no),
>>>>>>>>>> packets:9, bytes:882, used:0.125s, actions:ct(commit),recirc(0x4)
>>>>>>>>>> - [tc ]
>>>>>>>>>> recirc_id(0x4),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no),
>>>>>>>>>> packets:0, bytes:0, used:never, actions:3
>>>>>>>>>
>>>>>>>>> Hmm, but if the flow is in TC, why it is not dumped back and
>>>>>>>>> revalidated?
>>>>>>>>> It shouldn't matter if it has any traffic or not.
>>>>>>>>
>>>>>>>> Not sure I understand your comment. It’s revalidated and removed in
>>>>>>>> the end (as no traffic is hitting this rule). But if traffic comes
>>>>>>>> again, we install it in TC again, so the same problem repeats.
>>>>>>>
>>>>>>> The question is: if the flow is in TC, why it is not showing up in
>>>>>>> the detrace output? If it's in TC, it means we have the ukey and
>>>>>>> it must have been revalidated at least once.
>>>>>>
>>>>>> Ah, that part :) Yes, that got me puzzled too. But we do not populate
>>>>>> the xcache if there has been no traffic for this flow (which makes
>>>>>> sense):
>>>>>
>>>>> No, no. The flow is always revaliadated when it is dumped for the first
>>>>> time,
>>>>> regardless of the traffic:
>>>>>
>>>>> if (!used) {
>>>>> /* Always revalidate the first time a flow is dumped. */
>>>>> return true;
>>>>> }
>>>>>
>>>>> Or is need_revalidate == false in this case?
>>>>
>>>> Yes, this is the case, as this is only true if there is a reason to
>>>> revalidate, i.e. udpif_revalidate() was called.
>>>
>>> OK, but why this works for non-TC flows then?
>>
>> Because they have traffic. Sorry.
>
> I suppose, one easy fix for that can be to revalidator/wait, then
> make some benign unrelated change in OF rules, then wait again.
> The flow should be revalidated after that in all the cases.
>
> What do you think?
Yes, that should work, as I see in my failures that no revalidation is
happening. So, forcing this will fix the problem of this test for now.
>>>
>>>>
>>>>>> revalidate_ukey()
>>>>>>
>>>>>> 2475 }
>>>>>> 2476 } else if (!push.n_packets || ukey->xcache
>>>>>> 2477 || !populate_xcache(udpif, ukey, push.tcp_flags)) {
>>>>>> 2478 result = UKEY_KEEP;
>>>>>> 2479 }
>>>>>>
>>>>>> Why it is working, most of the time in my setup, is because an actual
>>>>>> revalidation is happening, and then we do populate the cache
>>>>>> independently of received packets (through revalidate_ukey__). Same
>>>>>> function;
>>>>>>
>>>>>> 2462 if (need_revalidate) {
>>>>>> 2463 if (should_revalidate(udpif, ukey, push.n_packets)) {
>>>>>> 2464 if (!ukey->xcache) {
>>>>>> 2465 ukey->xcache = xlate_cache_new();
>>>>>> 2466 } else {
>>>>>> 2467 xlate_cache_clear(ukey->xcache);
>>>>>> 2468 }
>>>>>> 2469 result = revalidate_ukey__(udpif, ukey, push.tcp_flags,
>>>>>> 2470 odp_actions, recircs,
>>>>>> ukey->xcache,
>>>>>> 2471 del_reason);
>>>>>> 2472 } else {
>>>>>> 2473 /* Delete, since it is too expensive to revalidate. */
>>>>>> 2474 *del_reason = FDR_TOO_EXPENSIVE;
>>>>>> 2475 }
>>>>>>
>>>>
>>>
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
