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.

> 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.

> 
> 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

Reply via email to