Operating System: All
Platform: All

Working on the patch.

@Jaime: Could you try one thing with this? When you see interfaces show up in child but not in interface state or in flows, can you try reconnecting switch [OF controller] to ODL?

I had tested this code and it was working when I first pushed it. I believe at some point other changes, namely Alon's parentRef fixes, modified the timing in such a way that it is not working. Looking at the code we update interface states based on child-interface information. I have rest of the changes ready. If this workaround works for you, then it means my understanding is correct and no more work needed on my patch.

https://git.opendaylight.org/gerrit/#/c/54251/

Tested update in interface and tunnel state. Couldn't test datapath in my setup.

I have removed and added the controller:

ovs-vsctl del-controller br-int
ovs-vsctl set-controller br-int tcp:172.28.128.1:6653

and nothing really changed in interfaces-state, tunnels still missing.

Also tried stop and starting the controller and all interfaces-state disappeared and con duplicate flow tunnels in OVS:

f3f6ea90-929f-4c77-96fe-bf891ddedba6
Manager "tcp:172.28.128.1:6640"
is_connected: true
Bridge br-int
Controller "tcp:172.28.128.1:6653"
is_connected: true
fail_mode: secure
Port "tap52b21189-17"
Interface "tap52b21189-17"
Port "tuncad4808d23e"
Interface "tuncad4808d23e"
type: vxlan
options:

ovs_version: "2.6.1"

Yeah, I noticed another bug in state code and have pushed a patch that fixes everything. I've tested it locally. Can you try out the patch in my previous comment?

Using both

https://git.opendaylight.org/gerrit/#/c/54251/
https://git.opendaylight.org/gerrit/#/c/54112/

just in case.

I managed to get one operational entry:

—

> {
> "interfaces-state": {
> "interface": [
>

,
>

,
>

,
>

,
>

> ]
> }
> }

—

> {
> "interfaces": {
> "interface": [
>

,
> {
> "name": "1a75efd4-7e9a-4c3e-9440-652a9386f57d",
> "type": "ia{
> "interfaces": {
> "interface": [
>

,na-if-type:l2vlan",
> "odl-interface:l2vlan-mode": "trunk",
> "enabled": true,
> "odl-interface:parent-interface": "tap1a75efd4-7e"
> },
>

,
>

> ]
> }
> }

—

> {
> "interface-child-info": {
> "interface-parent-entry": [
> {
> "parent-interface": "tapc99797fe-21",
> "interface-child-entry": [
>

> ]
> },
> {
> "parent-interface": "tun50f6dea39ac",
> "interface-child-entry": [
>

> ]
> },
> {
> "parent-interface": "tune34ed77ee80",
> "interface-child-entry": [
>

> ]
> },
> {
> "parent-interface": "tap1a75efd4-7e",
> "interface-child-entry": [
>

> ]
> }
> ]
> }
> }

—

Trying to understand a bit of what is happenning so I can help you out more. I assume with this change:

> if
> (InterfaceManagerCommonUtils.getInterfaceStateFromCache(tunnelName)
> != null)

that there should be an operational entry for the parent tunnel. And an
operational entry requires a configuration entry, otherwise
validateTunnelPortAttributes wont pass. Bu tthe configuration entry for the parent tunnel is not created anywhere.

Also, there might be a race condition between the child entry creation in operational, and the port configuration in OVSDB (they go into different queues in BatchingUtils). When this happens, the parent tunnel inventory state add might not create the appropiate operational state for all child entries.

Hope it helps.

Ignore this part

> I assume with this change:
>
> > if
> >
> (InterfaceManagerCommonUtils.getInterfaceStateFromCache(tunnelName)
> > != null)

>
> that there should be an operational entry for the parent tunnel. And
> an
> operational entry requires a configuration entry, otherwise
> validateTunnelPortAttributes wont pass. Bu tthe configuration entry
> for the parent tunnel is not created anywhere.

No, this interface will not have entry in config or operational, only in child interface info. State will not be added but code for it will be triggered fo its child interfaces and those are ones we want. Parent will start showing up once we support a use case of being able to create tunnels without known remote points so services can bind to those and decide remote-ip themselves, option 2 discussed in the mail thread.

tun2ff14bc55ea should've also shown up in inventoryState. 54112 is not needed for this, that is just to get information from caches. The probability of race condition is really low because we add child info before adding to OSVDB. Actual state doesn't get created till port shows up on switch and we get notifiation back. Code is written in such a way that OVSDB port addition or OF state addition both can be received in any order.

Anyhow, we got CSIT coming up for OFTunnels, so won't really merge this till those are in place. Brady tested with 54112 and it seems to solve the SFC requirements. Any issues with these two together, I'll address before this is merged.

Vishal, unless I am understanding something wrong, I would not discard the possibility of a race condition so easily. There is no happens-before relationship between child entry creation and port creation (or the inventory state listener where the interface-state of child entries is added).

Just to test, I moved the child entry creation from the DEFAULT_CONFIG queue to the TOPOLOGY_CONFIG queue (InterfaceManagerCommonUtils::createInterfaceChildEntry), the same queue used for port creation and now I got both tun operational entries:

{
> "interfaces-state": {
> "interface": [
>

,
>

,
>

,
>

,
>

,
>

> ]
> }
> }

(In reply to Jaime Caamaño Ruiz from comment #9)
> Vishal, unless I am understanding something wrong, I would not discard the
> possibility of a race condition so easily. There is no happens-before
> relationship between child entry creation and port creation (or the
> inventory state listener where the interface-state of child entries is
> added).
>
> Just to test, I moved the child entry creation from the DEFAULT_CONFIG queue
> to the TOPOLOGY_CONFIG queue
> (InterfaceManagerCommonUtils::createInterfaceChildEntry), the same queue
> used for port creation and now I got both tun operational entries:

creatInterfaceChildEntry is used for both tunnel and non-tunnel interfaces, so we can't use that. Non-tunnel will not be created in topology_config, only interface_config. We use Interface_Config as key in DSJC to order events for a given itnerfae irrespective of which shard it is writing/reading on. Maybe your fix is correct for this case, but I haven't tried both patchs together or seen the issue [all tunnels came up for me correctly].

Yeah, I agree with you it is not the correct fix, just a quick way to test it out.

Maybe a child interface entry listener that would call OvsInterfaceStateAddHelper::addState.