On 4/29/21 9:46 AM, Han Zhou wrote: > > > On Wed, Apr 28, 2021 at 6:55 PM Han Zhou <[email protected] > <mailto:[email protected]>> wrote: >> >> This reverts commit 0ba2e1a2e2c413f057ac198e4c2bcf00ac40b19f. >> >> 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 just figured out the root cause of the warning. It is because of the > conditional monitoring of DPGs. When the lflow insertion is updated to > ovn-controller, the DPG that is used by the lflow is not monitored yet, > because of the commit [0], which made the lflow monitor condition more > relaxed than the DPGs. I am working on a patch [1] that just always monitors > all DPGs, to avoid the unnecessary extra wakeups and processing.
I'm not sure if this will actually save wakeups. In some cases it will, but it will increase number of wakeups in other cases. If all datapath groups are monitored unconditionally, all controllers will wake up at the moment new datapath added or removed from any datapath group or if there is any other change. What do you think? For the processing. lflow will not be considered therefore not much to process. And when dp group will be updated, only affected lflows will be considered. There should not be a big overhead for processing, right? > With the change, all system tests are passed, but there is a new case failure > in ovn.at <http://ovn.at> (3 HVs, 3 LS, 3 lports/LS, 1 LR -- ovn-northd -- > dp-groups=yes), and I am still debugging it. > > [0] d41a337fe3 "controller: Monitor all logical flows that refer to datapath > groups." > [1] > https://github.com/hzhou8/ovn/commit/bc8f4ab3cdaee0bc8c8abf5293e7356c2d13d48c > <https://github.com/hzhou8/ovn/commit/bc8f4ab3cdaee0bc8c8abf5293e7356c2d13d48c> > > Thanks, > Han > >> >> Cc: Ilya Maximets <[email protected] <mailto:[email protected]>> >> Signed-off-by: Han Zhou <[email protected] <mailto:[email protected]>> >> --- >> controller/lflow.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/controller/lflow.c b/controller/lflow.c >> index b8424e1fb..680b8cca1 100644 >> --- a/controller/lflow.c >> +++ b/controller/lflow.c >> @@ -916,9 +916,8 @@ consider_logical_flow(const struct sbrec_logical_flow >> *lflow, >> bool ret = true; >> >> if (!dp_group && !dp) { >> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); >> - VLOG_WARN_RL(&rl, "lflow "UUID_FMT" has no datapath binding, skip", >> - UUID_ARGS(&lflow->header_.uuid)); >> + VLOG_DBG("lflow "UUID_FMT" has no datapath binding, skip", >> + UUID_ARGS(&lflow->header_.uuid)); >> return true; >> } >> ovs_assert(!dp_group || !dp); >> -- >> 2.30.2 >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
