On 7/14/23 12:01, Ales Musil wrote: > On Fri, Jul 14, 2023 at 11:39 AM Xavier Simonart <[email protected]> > wrote: > >> Hi Ales >> >> Thanks for looking into this. >> >> On Fri, Jul 14, 2023 at 8:17 AM Ales Musil <[email protected]> wrote: >> >>> >>> >>> On Thu, Jul 13, 2023 at 1:08 PM Xavier Simonart <[email protected]> >>> wrote: >>> >>>> Signed-off-by: Xavier Simonart <[email protected]> >>>> --- >>>> >>> >>> Hi Xavier, >>> >>> >>>> tests/ovn.at | 22 +++++++++++++++++----- >>>> 1 file changed, 17 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/tests/ovn.at b/tests/ovn.at >>>> index cd6d4b9ff..4ae33567f 100644 >>>> --- a/tests/ovn.at >>>> +++ b/tests/ovn.at >>>> @@ -17058,7 +17058,7 @@ rtr_l2_ip=$(ip_to_hex 172 16 1 1) >>>> l1_ip=$(ip_to_hex 192 168 1 2) >>>> >>>> check ovn-nbctl mirror-add mirror0 gre 0 to-lport 192.168.1.12 >>>> -check ovn-nbctl lsp-attach-mirror ls1-lp1 mirror0 >>>> +check ovn-nbctl --wait=hv lsp-attach-mirror ls1-lp1 mirror0 >>>> >>>> # Send ping packet and check for mirrored packet of the reply >>>> test_ipv4_icmp_request 1 000000010203 0000000102f1 $l1_ip $rtr_l2_ip >>>> 0000 8510 03ff 8d10 "gre" "to-lport" >>>> @@ -17071,7 +17071,9 @@ as hv1 reset_pcap_file br-phys_n1 hv1/br-phys_n1 >>>> rm -f br-phys_n1.expected >>>> rm -f vif1.expected >>>> >>>> -check ovn-nbctl set mirror . type=erspan >>>> +check ovn-nbctl --wait=hv set mirror . type=erspan >>>> +# Wait for port to get updated >>>> +OVS_WAIT_UNTIL([test 1 = `ovs-appctl dpif/show | grep ovn-mirror | grep >>>> -c erspan`]) >>>> >>>> # Send ping packet and check for mirrored packet of the reply >>>> test_ipv4_icmp_request 1 000000010203 0000000102f1 $l1_ip $rtr_l2_ip >>>> 0000 8510 03ff 8d10 "erspan" "to-lport" >>>> @@ -17084,7 +17086,15 @@ as hv1 reset_pcap_file br-phys_n1 hv1/br-phys_n1 >>>> rm -f br-phys_n1.expected >>>> rm -f vif1.expected >>>> >>>> -check ovn-nbctl set mirror . filter=from-lport >>>> +check ovn-nbctl --wait=hv set mirror . filter=from-lport >>>> + >>>> +# First make sure conf.db got updated >>>> +vif1=$(ovs-vsctl get Port vif1 _uuid) >>>> +OVS_WAIT_UNTIL([test `ovs-vsctl get mirror mirror0 select_src_port | tr >>>> -d "[[]]"` = $vif1]) >>>> +# Then make sure ovs-vswitchd got opportunity to run : run some random >>>> ovs-apctl command twice, so >>>> +# mirror_run could run >>>> +ovs-appctl dpif/show >>>> +ovs-appctl dpif/show >>>> >>> >>> I'm not sure I understand what is the purpose of this "Then make sure >>> ovs-vswitchd got opportunity to run". >>> Could you please elaborate a bit? >>> >>> >> My understanding of those race conditions are the following - but I might >> be wrong >> When we run ovn-nbctl --wait=hv, there is no guarantee that, after >> ovn-nbctl returns, conf.db is already updated. >> If that ovsdb-server process is very slow (or not scheduled on a busy >> system), it might still take some time before the db gets properly updated. >> That's the reason for the OVS_WAIT_UNTIL above. >> But even after the db gets updated, ovs-vswitchd has to get notified of >> that db change and run mirror_configure (sorry the above comment on >> mirror_run was wrong) to reconfigure the mirror. >> And, again, it might take a while for ovs-vswitchd process to get >> scheduled on a busy system. >> The only way I found to ensure that ovs-vswitchd had run at least one loop >> is to have some appctl command talking to ovs-vswitchd (i.e. ovs-appctl >> dpif/show does not show the change, and I did not find any other >> appctl/vsctl command to do show it). >> I am not sure that "one" ovs-appctl would be enough as it does not >> guarantee the run of a full loop in ovs-vswitchd. >> This is really hacky, but I do not know any way. >> Do you agree with that analysis, and, if yes, do you have any better way >> to fix this ? >> > > Ok that is reasonable, unfortunately I don't have a better idea how to > achieve that. > Would you mind putting this analysis into the commit message so we have it > documented? >
Thanks, Xavier and Ales! I updated the commit message accordingly and pushed this patch to main and backported it to 23.06. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
