On 4/29/21 11:29 AM, Ilya Maximets wrote: > On 4/29/21 10:07 AM, Dumitru Ceara wrote: >> On 4/29/21 3:55 AM, Han Zhou wrote: >>> This reverts commit 0ba2e1a2e2c413f057ac198e4c2bcf00ac40b19f. >> >> Oops, I should've thought about this when reviewing the initial patch, >> good that the CI caught it. >> >>> >>> System tests with "dp-groups=yes" are all failing due to the warning >>> log. Need to investigate why the warning appears. But before that is >>> clear, revert this log level change to make CI pass. >> >> I would remove this part of the commit log or change it to something like: >> >> "With conditional monitoring, if a logical flow refers to a datapath >> group that doesn't contain any local datapaths, ovsdb-server will send >> updates with lflow.logical_dp_group set to NULL. > > This is not correct. lflow does have a logical_dp_group in a message > received from the ovsdb-server. It's IDL that hides it from a user > because it didn't receive the group itself. And it's allowed to do > that because of the 'min:0, max:1' definition of the column in the schema.
You're right, thanks for the correction. > >> This is a valid case >> and we shouldn't log a warning message for it." > > +1 > It's a valid case and should not be logged as a warning. > >> >> It's actually expected to have lflows with both logical_dp_group and >> logical_datapath NULL now since we unconditionally monitor lflows with >> datapath group set. > > Another point here is that we can't actually guarantee the order of updates > from the server. Even though currently ovsdb-server sends updates for > monitored tables in a single 'update' message, it's not mandatory and could > change in the future. In this case it will be possible to receive lfow > before receiving dp_group even with monotor-all. +1 So even with the additional patch Han is preparing (to unconditionally monitor all DPGs) we should still revert this change. > >> >> With conditional monitoring it might be that there are lflows that refer >> to a DPG but none of the DPG datapaths are local to ovn-controller so >> they won't be monitored. In that case lflow.logical_dp_group will be >> NULL. It's not a bug however so the old VLOG_DBG was fine. >> >> For the revert patch: >> Acked-by: Dumitru Ceara <[email protected]> >> >> Thanks, >> Dumitru >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
