Can you try this again with this patch:https://git.opendaylight.org/gerrit/#/c/62272/ ?
I tested patch set 3 of this patch [0], and no changes were observed. That is, the problem has not been fixed.
Okay, uploaded new patch, problem was probably with inconsisten extension augmentation class names, so swapped comparison of matches by equal method (what is also checking for classnames) with comparison of matches by hashCode (what is only comparing contents of objects), what should solve this issue, because contents of augmentations are same, but wrapping classes are not.
I tested with patch set 4, and no changes were observed. This is still failing.
Looks like the proposed patch was abandoned:
https://git.opendaylight.org/gerrit/#/c/62272
Will another patch be proposed then?
So as my patch was not solving the issue, it looks like that with extensions there isn't properly working matching of flows, and my patch do not solved that, so I abandoned it. Currently I moved to blocked bugs, so I am not working on another patch that can solve this issue.
This bug should have been a blocker from the beginning, since its blocking the SFC statistics collection feature. Because of this bug, I wasnt able to complete the feature in Nitrogen and I cant progress in Oxygen.
The result of this failure is inconsistent flow information between the config and operational data store, making the operational flows basically useless since they cant be queried, which is a blocker in and of itself.
For those 2 reasons, Im changing this bug to be a blocker.
Okay, as there are currently only pending patches I can look at this issue again. I actually found one issue with patch that I created for this issue. I will un-abandon the patch and make small edit in few mins. Can you test that again after the change please?
Ok, thanks for looking again.
Yes, I can test that again today.
I still get the following:
$ curl -H "Content-Type: application/json" -X GET --user admin:admin http://localhost:8181/restconf/config/opendaylight-inventory:nodes/node/openflow:156930464280132/table/4 | python -m json.tool | grep \"id\"
"id": "matchAny_4_10",
"id": "nextHop_255_8388639",
"id": "nextHop_255_31",
"id": 4
curl -H "Content-Type: application/json" -X GET --user admin:admin http://localhost:8181/restconf/operational/opendaylight-inventory:nodes/node/openflow:156930464280132/table/4 | python -m json.tool | grep \"id\"
"id": "matchAny_4_10",
"id": "#UF$TABLE*4-3",
"id": "#UF$TABLE*4-4",
"id": 4,
Okay I tried to reproduce this on my local machine and I keep getting BADMATCH BADFIELD from OVS. Do I need some special OVS build to have this work? I have OVS 2.7.2 running rn.
I think I found the culprit, created custom hashing function for match, can you try this again? Since i cannot try it locally.
This is still not working with patch-set 9.
To reproduce this yourself, you'll need a version of OVS that supports NSH (Network Service Headers) that you can find here:
Simple steps to reproduce this yourself without SFC, you'll just need the openflow plugin:
1) Create an OVS bridge and set the controller to ODL
sudo ovs-vsctl add-br br-int
sudo ovs-vsctl set-manager "tcp:<ODL IP>:6640"
sudo ovs-vsctl set-controller "tcp:<ODL IP>:6653"
2) Get the openflow ID:
curl -H "Content-Type: application/json" -X GET --user admin:admin http://localhost:8181/restconf/operational/opendaylight-inventory:nodes/ | python -m json.tool | grep "openflow:"
"id": "openflow:156930464280132",
"id": "openflow:156930464280132:1",
"id": "openflow:156930464280132:LOCAL",
2) Create the flow:
(notice, replace openflow:156930464280132 accordingly)
curl -H "Content-Type: application/json" -X PUT --user admin:admin --data '{ "flow": [ { "barrier": false, "cookie": 20, "cookie_mask": 20, "flags": "", "flow-name": "nextHop_255_300", "hard-timeout": 0, "id": "nextHop_255_300", "idle-timeout": 0, "instructions": { "instruction": [ { "apply-actions": { "action": [ { "openflowplugin-extension-nicira-action:nx-reg-load": { "dst":
{ "end": 31, "nx-tun-ipv4-dst": [ null ], "start": 0 }, "value": 167772170 }, "order": 0 } ] }, "order": 0 }, { "go-to-table":
{ "table_id": 10 }, "order": 1 } ] }, "match": { "openflowplugin-extension-general:extension-list": [ { "extension": { "openflowplugin-extension-nicira-match:nxm-nx-nsp":
{ "value": 300 }}, "extension-key": "openflowplugin-extension-nicira-match:nxm-nx-nsp-key" }, { "extension": { "openflowplugin-extension-nicira-match:nxm-nx-nsi":
{ "nsi": 255 }}, "extension-key": "openflowplugin-extension-nicira-match:nxm-nx-nsi-key" } ] }, "priority": 550, "strict": false, "table_id": 4 } ] }' http://localhost:8181/restconf/config/opendaylight-inventory:nodes/node/openflow:156930464280132/flow-node-inventory:table/4/flow/nextHop_255_300
3) Query the flow-id in config
(notice, replace openflow:156930464280132 accordingly)
curl -H "Content-Type: application/json" -X GET --user admin:admin http://localhost:8181/restconf/config/opendaylight-inventory:nodes/node/openflow:156930464280132/table/4 | python -m json.tool | grep "\"id\"\:"
"id": "nextHop_255_300",
"id": 4
4) Query the flow-id in operational
(notice, replace openflow:156930464280132 accordingly)
curl -H "Content-Type: application/json" -X GET --user admin:admin http://localhost:8181/restconf/operational/opendaylight-inventory:nodes/node/openflow:156930464280132/table/4 | python -m json.tool | grep "\"id\"\:"
"id": "#UF$TABLE*4-1", <== This is wrong
"id": 4,
Moved to Anil (from Brady / Tomas).
I am able to recreate this issue locally with OVS 2.6.90 + NSH patches. Debugging the issue now.
it's an issue related to object hashcode values for the augmentations. To me it looks like this issue will happen for any augmentation on match. In am able to recreate it by augmenting single extension on the flow match
"match": {
"openflowplugin-extension-general:extension-list": [
{
"extension": {
"openflowplugin-extension-nicira-match:nxm-nx-nsp": {
"value": 300
}
},
"extension-key": "openflowplugin-extension-nicira-match:nxm-nx-nsp-key"
}
]
},
Plugin receives the data change notification when user write flow to data store and uses that data change notification data to generate the flow body (match, instruction) and dump it to switch. And looks like the notifications data is populated lazily and and plugin call the hashcode on Match object ( augmented with the extension) it return different hashcode value, compared to the object build through the normal builder object.
Augmentation on Match object created using the data change notification data
2017-10-25 12:51:06,532 | INFO | on-dispatcher-43 | FlowRegistryKeyFactory | 285 - org.opendaylight.openflowplugin.impl - 0.5.1.SNAPSHOT | Augmentation : GeneralAugMatchNodesNodeTableFlow{getExtensionList=[ExtensionList{getExtension=Extension{augmentations={interface org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.match.rev140714.NxAugMatchNodesNodeTableFlow=NxAugMatchNodesNodeTableFlow{getNxmNxNsp=NxmNxNsp{getValue=300, augmentations={}}}}}, getExtensionKey=class org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.match.rev140714.NxmNxNspKey, augmentations={}}]} *hashcode : -745547690
*
Augmentation on Match created through builders provided by java bindings
2017-10-25 12:51:09,241 | INFO | entLoopGroup-7-1 | FlowRegistryKeyFactory | 285 - org.opendaylight.openflowplugin.impl - 0.5.1.SNAPSHOT | Augmentation : GeneralAugMatchNodesNodeTableFlow [_extensionList=[ExtensionList [_extension=Extension [augmentation=[NxAugMatchNotifUpdateFlowStats [_nxmNxNsp=NxmNxNsp [_value=300, augmentation=[]], ]]], _extensionKey=class org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.match.rev140714.NxmNxNspKey, _key=ExtensionListKey [_extensionKey=class org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.match.rev140714.NxmNxNspKey], augmentation=[]]]]* hashcode : -1428243401*
It's a same augmentation with the same values, but we see different hashcode value when it comes from data store and through builder object, and that's where plugin thinks it's not a match and store it as a alien flow. It can be workaround by creating builder object for all the augmentation data received through data change notification, but that's not a practical approach because openflowplugin provides the augmentation for vendors to hook their extensions, and everytime someone hook their augmentation, plugin code need to be changes. I am sending my findings to mdsal/yangtools team to provide the feedback on this.
ebrjohn you mentioned that it only happens to nsh extension flows, but looking at the code it should happen for any flow that augment extension on the match. If you have a flow handy, can you give me a flow that uses other extension and it's working for you. I tried with the following basic nicira extension flow and it was failing as well
<match>
<ethernet-match>
<ethernet-type>
<type>2048</type>
</ethernet-type>
</ethernet-match>
<ipv4-destination>10.0.0.0/24</ipv4-destination>
<extension-list xmlns="urn:opendaylight:openflowplugin:extension:general">
<extension-key xmlns:nxNB="urn:opendaylight:openflowplugin:extension:nicira:match">nxNB:nxm-nx-reg0-key</extension-key>
<extension>
<nxm-nx-reg xmlns="urn:opendaylight:openflowplugin:extension:nicira:match">
<reg xmlns:nxSB="urn:opendaylight:openflowjava:nx:match">nxSB:nxm-nx-reg0</reg>
<value>42</value>
</nxm-nx-reg>
</extension>
</extension-list>
</match>
I pushed the following patch and it fixes the issue. I tested with the provided flow and it looks good.
Please get the patch merged and we can start a release candidate build for Nitrogen-SR1. Thanks!
Im testing this with SFC now.
I tested this in Nitrogen, and it is working as expected 
so Im ok with merging the patch in Nitrogen.
Avishnoi Regarding your previous request to test this with flows that have augment extensions that arent nsh extension flows: there arent any such flows in SFC.
https://git.opendaylight.org/gerrit/64913
https://git.opendaylight.org/gerrit/64914
ecelgp can we add a test in our CSIT test to capture this bug?
Make sure your patch is merged in all active branches otherwise this will fail:
Yup, all the patches are now merged.
I guess we can close this then.