On 7/14/26 2:29 PM, Ilya Maximets wrote:
> On 7/13/26 4:40 PM, David Marchand wrote:
>> On Mon, 13 Jul 2026 at 16:08, Ilya Maximets <[email protected]> wrote:
>>>
>>> The test creates an OpenFlow pipeline with deeply nested resubmits
>>> in order to trigger packet drops with the 'too many resubmits' reason.
>>>
>>> However, it doesn't use bundles and the priorities on the rules are
>>> the same while matches can technically overlap between the in_port
>>> ones and the dl_type ones.  So, if a packet, e.g., ICMPv6 ND, enters
>>> OVS while the pipeline is not fully configured yet, this packet can
>>> hit only a subset of resubmits that doesn't reach the limit and it
>>> will be sent many times to the local port.  This could confuse the
>>> system and mess up counters, as well as create a lot of unnecessary
>>> traffic.
>>>
>>> Let's set the rules up more carefully with proper priorities and
>>> passing ICMPv6 traffic via normal pipeline, so only ICMP packets
>>> generated by the ping will hit the resubmits.  Also using a bundle,
>>> so packets never hit a partially configured pipeline.
>>>
>>> Fixes: edf56245b828 ("tests: system-traffic: Add coverage for drop action.")
>>> Signed-off-by: Ilya Maximets <[email protected]>
>>
>> I would have written the rules in the other way: narrow down the
>> resubmit rule with a priority, and a icmp filter, and let in place the
>> default rule with normal action.
> 
> Yeah, there are a few ways to get to a similar result, it doesn't
> matter much.
> 
>> But this current form looks good to me.
>>
>> Acked-by: David Marchand <[email protected]>
> 
> Thanks.  There is some other issue with this test, I got a different
> failure with this patch applied - dumped datapath flows have
> used:never and the expected output doesn't match because of this.
> Need to take a closer look and send v2 or a separate patch.  But it's
> not related to the dynamic batches patch set.

I could not reproduce this failure, the only possible case that I see
how it could've happened is if the main thread was delayed enough in
CI that it managed to process all incoming ICMP traffic in a single
batch, or at least very close to each other.  This way the installed
datapath flow will not have a chance to get any hits accounted.
But this is unrelated to this particular patch or test, so should be
addressed separately.  I'll keep looking into it.

For now, I applied this patch to main and branch-3.7.  We could backport
further later, if needed.  Thanks!

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to