On Mon, Aug 28, 2023 at 4:19 AM Ilya Maximets <[email protected]> wrote: > > On 8/25/23 08:12, Mike Pattrick wrote: > > On Thu, Aug 24, 2023 at 6:01 PM Ilya Maximets <[email protected]> wrote: > >> > >> On 8/22/23 22:45, Mike Pattrick wrote: > >>> When the a revalidator thread is updating statistics for an XC_LEARN > >>> xcache entry in xlate_push_stats_entry it uses ofproto_flow_mod_learn. > >>> The revalidator will update stats for rules even if they are in a > >>> removed state or marked as invisible. However, ofproto_flow_mod_learn > >>> will detect if a flow has been removed and re-add it in that case. This > >>> can result in an old learn action replacing the new learn action that > >>> had replaced it in the first place. > >>> > >>> This change adds a new stats_used parameter to ofproto_flow_mod_learn > >>> allowing the caller to provide a timestamp that will be fed into the > >>> learned rule's modified time. The provided timestamp should be the time > >>> of the last packet activity. If stats_used is not set then the current > >>> time is used, as is the current behaviour. > >>> > >>> This change also adds a check when replacing a learned rule to favour the > >>> newest rule. > >>> > >>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2213892 > >>> Signed-off-by: Mike Pattrick <[email protected]> > >>> --- > >>> v2: Added additional checks if rule is removed > >>> v3: v2 patch was corrupted in transit > >>> v4: Added check against dpif flow stats > >>> v5: Fixed typos, updated commit message > >>> Changed timestamps to use datapath timestamp more consistently > >>> --- > >> > >> I'll quote my previous review comment: > >> > >> It should be possible to add a test for this issue using time > >> warping. Please, add one. > > > > I tried to make a test with netdev-dummy/receive and time/warp, > > however, it didn't reproduce the behaviour that's observed with a full > > test. Do you have any suggestions? > > It's hard to guess what went wrong with your test without looking at it. > But the following script reproduces the behavior consistently for me > in a sandbox:
Thanks for that. I missed pausing/resuming the revalidator. Will send in a revised version with your comments addressed. -M > > ovs-vsctl show > ovs-vsctl add-br br0 > ovs-vsctl add-port br0 p0 > ovs-vsctl add-port br0 p1 > ovs-vsctl add-port br0 p2 > ovs-ofctl del-flows br0 > ovs-ofctl add-flow br0 > 'in_port=p1,actions=learn(cookie=0xabc,in_port=p0,output:in_port),p0' > ovs-ofctl add-flow br0 > 'in_port=p2,actions=learn(cookie=0xabc,in_port=p0,output:in_port),p0' > ovs-ofctl dump-flows br0 > > packet='recirc_id(0),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff)' > > # Stop the time, so flows do not expire. > ovs-appctl time/stop > # Send 2 packets to create learning datapath flows. > ovs-appctl netdev-dummy/receive p1 $packet > ovs-appctl netdev-dummy/receive p2 $packet > ovs-appctl revalidator/wait > > # Running twice in a loop to be sure we're not just lucky > # with revalidation order. > for i in 1 2; do > # Pause revalidators, so they will revalidate both datapath flows > # in the same revalidation cycle next time. > ovs-appctl revalidator/pause > ovs-appctl time/warp 10 > # Send a packet on p1, then on p2 after 10 ms. > ovs-appctl netdev-dummy/receive p1 $packet > ovs-appctl time/warp 10 > ovs-appctl netdev-dummy/receive p2 $packet > ovs-appctl time/warp 10 > # Un-pause and wait for revalidation. > ovs-appctl revalidator/resume > ovs-appctl revalidator/wait > > echo "Should be p2 ?" > ovs-ofctl dump-flows br0 'cookie=0xabc/0xfff' > > # Do the same, but send packets in the opposite order. > ovs-appctl revalidator/pause > ovs-appctl time/warp 10 > ovs-appctl netdev-dummy/receive p2 $packet > ovs-appctl time/warp 10 > ovs-appctl netdev-dummy/receive p1 $packet > ovs-appctl time/warp 10 > ovs-appctl revalidator/resume > ovs-appctl revalidator/wait > > echo "Should be p1 ?" > ovs-ofctl dump-flows br0 'cookie=0xabc/0xfff' > done > > Best regards, Ilya Maximets. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
