On 27/11/2018 18:19, Vishal Thapar wrote:

Hi Robert, Tom,

Hello Vishal,

Pardon my ignorance, I am not familiar with the mechanics/models here.

We're seeing an issue where Genius and Netvirt code both are adding a
TerminationPoint/port to an OVSDB bridge. TerminationPoint is a keyed
list where port name is the key. IID used in both cases is properly
keyed to port name. This always happens when Genius port is the first
port added to bridge. If Netvirt code is the one adding first port, all
works fine.

"port" the same thing as TerminationPoint, right?

The issue we're seeing is that any ports added by Genius before Netvirt
adds the port are deleted when Netvirt adds the port. Tim has a
workaround [1] that seems to work. It just replaces put with a merge in
the method called by Netvirt.

I suck at explaining, so bear with me :) The data being put/merged is
the same:

TP {
key=TpId(portName)
augmentations={OvsdbTerminationPointAugmentation}
}

If the TP does not exist, the two operations have the same outcome: the
termination point is created and DTCL will see a ModificationType.WRITE
on the IID(TP) path.

If the TP does exist, though, there is a difference:

put() will have the same effect as above and it will replace the TP
content (augmentations, other fields) with the contents of that field.
If you read the IID(TP), you will get back the same object.

merge() will have a different effect:
- it will retain any present data in the TP and only replace
non-matching specified items: in this case it will have the same effect
as put(IID(TP)/OvsdbTerminationPointAugmentation)
- that means that any existing augmentations will be retained, so if you
read IID(TP) you will see your augmentation and anything else that was
there.
- it also means DTCLs will see a ModificationType.SUBTREE_MODIFIED on
the IID(TP) and will see ModificationType.WRITE on
IID(TP/OvsdbTerminationPointAugmentation)

But genius code is also doing a put and that works, it doesn't delete
any existing ports. In fact Genius code most times adds multiple ports
in quick succession. Only difference between two is Genius code sets
createMissingParents=True.

Got a pointer to that code?

Assuming bridgeNode exists in the data store, createMissingParents=true
should not have a bearing on the effects of the overall list (but I may
be missing something or there could be a bug).

Any inputs on why Netvirt method ends up overwriting the existing ports
in list and why merge works? Want to understand if we're missing
something crucial and there can be a better fix than changing merge to put.

By overwriting, do you mean other entries in the TerminationPoint list
disappear? If so, does that happen in the same transaction that
introduced the netvirt port? (that sounds like a bug)

Or could this be Genius reacting to the difference in DTCL explained above?

Do the port names overlap at all?

Thanks,
Robert