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.

> 
>>
>>>> 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

Reply via email to