On 4/20/20 16:57, Pier Luigi Ventre wrote:
> Hi everyone.
> 
> This patch is a followup to this message sent in ovs-discuss:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-April/049897.html 
> <https://mail.openvswitch.org/pipermail/ovs-discuss/2020-April/049897.html>
> 
> This is the brief description of the change:
> 
> The patch fixes ofgroup ref_count in the group stats. Previously, only the 
> rules were counted.
> However, ofgroups can reference other ofgroups.
> 
> Updates tests that were failing: groups were not created in order. Adds new 
> unit tests for reference count
> 
> Signed-off-by: Pier Luigi Ventre <[email protected].>

Hi Pier,

I was looking through old patches in our patchwork instance and
found this one.  Sorry for not looking at it earlier.

It does look like an issue in our implementation for reference
counters indeed.

There are few issues with the implementation though.  The code
doesn't check if the group referenced in the bucket exists
while adding a new group.  This will skew the counting down the
road when the group will be added later.
Also, there is no mechanism to protect us from removing the
group that is still referenced by some other group.  OVS will
just assert instead.  That is not good.

One way to fix these issues is to actually take real reference
and forbid adding groups that reference non-existent groups
or removing groups that are still referenced.  I'm not sure
though how that align with the OpenFlow spec and that will
for sure impose extra restrictions on the order of operations
for existing users.

Another option is to find a way to count things while keeping
in mind that some groups may not actually exist.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to