Anil, As far as I can tell there's just one left in the IT that is not showing up. The so called TunnelMiss_<seg_id> flow that is present on the switch.: cookie=0x0, duration=3.296s, table=110, n_packets=0, n_bytes=0, priority=8192,tun_id=0x65 actions=drop I can't see any obvious reason why this would not work so if you'd like I can try and debug it using the IT test.
-J On Sun, Jul 31, 2016 at 8:03 PM, Anil Vishnoi <[email protected]> wrote: > josh, is it possible to get the flow details for these flows? that will > give us some idea why it's failing. > > On Tue, Jul 26, 2016 at 12:50 AM, Josh Hershberg <[email protected]> > wrote: > >> So it does not work well enough, see this output [1]. I have not invested >> too much time in analyzing why these failures have occurred although some >> of them look rather suspicious, especially DEFAULT_PIPELINE_FLOW_70. >> >> [1] >> Failed tests: >> NetvirtIT.testNetVirt:421 Could not find flow in operational: Flow >> [_flowName=DEFAULT_PIPELINE_FLOW_70, _hardTimeout=0, _id=Uri >> [_value=DEFAULT_PIPELINE_FLOW_70], _idleTimeout=0, _key=FlowKey [_id=Uri >> [_value=DEFAULT_PIPELINE_FLOW_70]], _tableId=70, _barrier=false, >> _strict=true, augmentation=[]]--Node [_id=Uri >> [_value=openflow:106208305082963], _key=NodeKey [_id=Uri >> [_value=openflow:106208305082963]], augmentation=[]] >> NetvirtIT.testNetVirtFixedSG:493 Could not find flow in operational: >> Flow [_flowName=Egress_DHCP_Client_Permit_, _hardTimeout=0, _id=Uri >> [_value=Egress_DHCP_Client_Permit_], _idleTimeout=0, _key=FlowKey [_id=Uri >> [_value=Egress_DHCP_Client_Permit_]], _tableId=40, _barrier=false, >> _strict=true, augmentation=[]]--Node [_id=Uri >> [_value=openflow:106208305082963], _key=NodeKey [_id=Uri >> [_value=openflow:106208305082963]], augmentation=[]] >> NetvirtIT.testNeutronNet:640 Could not find flow in operational: Flow >> [_flowName=TunnelMiss_101, _hardTimeout=0, _id=Uri [_value=TunnelMiss_101], >> _idleTimeout=0, _key=FlowKey [_id=Uri [_value=TunnelMiss_101]], >> _tableId=110, _barrier=false, _strict=true, augmentation=[]]--Node [_id=Uri >> [_value=openflow:106208305082963], _key=NodeKey [_id=Uri >> [_value=openflow:106208305082963]], augmentation=[]] >> >> >> On Tue, Jul 26, 2016 at 4:26 AM, Anil Vishnoi <[email protected]> >> wrote: >> >>> Given that we crossed the M5, and the time we have in hand, long term >>> solution of generating equals/hashcode and getAllAugmentation doesn't look >>> feasible. So in my opinion, this looks like a good short term solution. >>> >>> On Mon, Jul 25, 2016 at 7:23 PM, Josh Hershberg <[email protected]> >>> wrote: >>> >>>> Will do a little later. However, our current code does not set cookies >>>> and if we do not really think that is the correct long term solution, are >>>> we sure we want it now? >>>> >>>> On Tue, Jul 26, 2016 at 1:15 AM, Anil Vishnoi <[email protected]> >>>> wrote: >>>> >>>>> So i hooked up the He plugin custom comparator code to the Li plugin - >>>>> FlowRegistryKeyFactory. >>>>> Custom comparator will only trigger if it won't find the >>>>> FlowRegistryKey in the flowRegistery TrieMap. >>>>> With this all the operational flow related regressions will be address. >>>>> >>>>> Josh, can you please test with this patch and see if it resolves the >>>>> issue. It does not use augmentation comparator, rather it uses cookie >>>>> value >>>>> in comparison, so as far as you are using unique cookie, operational flow >>>>> should be present at the correct location in operational data store. >>>>> >>>>> Anil >>>>> >>>>> On Mon, Jul 25, 2016 at 9:10 AM, Abhijit Kumbhare < >>>>> [email protected]> wrote: >>>>> >>>>>> Sounds good (discussing over email till the meeting on Thursday). >>>>>> >>>>>> On Sun, Jul 24, 2016 at 1:14 AM, Anil Vishnoi <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> I just realised that next meeting is on Thursday (still i feel >>>>>>> Openflowplugin meeting is on Monday), which is i think going to be bit >>>>>>> late. Can we discuss on the short term plan in this mail thread so that >>>>>>> we >>>>>>> can fix it on time ? There are couple of more bugs that is reported >>>>>>> related >>>>>>> to the same issue as well. >>>>>>> >>>>>>> On Sun, Jul 24, 2016 at 12:55 AM, Anil Vishnoi < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Abhijit, can we please put this on agenda for next weeks meeting, >>>>>>>> we need to resolve this issue because it probably will surface many >>>>>>>> regressions with Li plugin. >>>>>>>> >>>>>>>> On Sun, Jul 24, 2016 at 12:54 AM, Anil Vishnoi < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Wed, Jul 20, 2016 at 8:28 PM, Josh Hershberg < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> Yeah. >>>>>>>>>> >>>>>>>>>> The Problem >>>>>>>>>> ======================== >>>>>>>>>> Openflowplugin has a "registry" of flows within which flows are >>>>>>>>>> stored in a hash table. When a flow comes in an update (OFST_FLOW) >>>>>>>>>> it does >>>>>>>>>> not have the flow id assigned to it when it was created via the >>>>>>>>>> md-sal. >>>>>>>>>> Openflowplugin looks up the flow in the registry to find the id of >>>>>>>>>> the >>>>>>>>>> flow. The problem is, that there are multiple representations of the >>>>>>>>>> same >>>>>>>>>> flow in our yang models and depending on where and how the flow >>>>>>>>>> entered the >>>>>>>>>> system it will be represented syntactically different but >>>>>>>>>> semantically >>>>>>>>>> identical. As far as I could tell the syntactic differences were >>>>>>>>>> around >>>>>>>>>> "container" structures, not the Match objects themselves. The >>>>>>>>>> .hashCode and >>>>>>>>>> .equals methods generated for the md-sal objects are effected by the >>>>>>>>>> syntactic elements and two semantically identical flows will have >>>>>>>>>> different >>>>>>>>>> .hashCodes and will not be .equals and will therefor not be found in >>>>>>>>>> the >>>>>>>>>> registry and added to the OPERATIONAL data store with "alien" flow >>>>>>>>>> IDs. You >>>>>>>>>> can see an example of the toString'ed md-sal objects and how they >>>>>>>>>> differ >>>>>>>>>> syntactically but are identical semantically in the description of >>>>>>>>>> the bug >>>>>>>>>> [1]. >>>>>>>>>> >>>>>>>>>> HE Solution [2] >>>>>>>>>> ======================== >>>>>>>>>> In the He plugin the solution was to write a custom compare >>>>>>>>>> function (I assume there's a hash too but didn't check). The compare >>>>>>>>>> function manually compares the fields of the match which is what the >>>>>>>>>> generated .equals function does but with one big difference, the >>>>>>>>>> custom >>>>>>>>>> compare function skips any extension Match objects. >>>>>>>>>> >>>>>>>>> Below problem is solved by adding cookie in the flow comparison, >>>>>>>>> so if you use different cookie value for these flows, their >>>>>>>>> operational >>>>>>>>> flow will be augmented at the correct id. It's not the solution, but >>>>>>>>> rather >>>>>>>>> a workaround thats been used in HE plugin. >>>>>>>>> >>>>>>>>> >>>>>>>>>> What this means is that two flows that differ only in an >>>>>>>>>> extenstion match, e.g., NxmNxTunIpv4DstGrouping, will be considered >>>>>>>>>> equal >>>>>>>>>> and one will overwrite the other in OPERATIONAL. >>>>>>>>>> >>>>>>>>>> My Patch [3] >>>>>>>>>> ======================== >>>>>>>>>> I pushed a patch that walks the augmentations and compares the >>>>>>>>>> match objects themselves, ignoring the containers but comparing the >>>>>>>>>> extension Match objects. It does this by looping through a hard >>>>>>>>>> coded list >>>>>>>>>> of augmentations and then comparing the Match objects contained >>>>>>>>>> therein. >>>>>>>>>> Jozef Becigal pointed out, correctly, that this will break if there >>>>>>>>>> is a >>>>>>>>>> new extension Match added because the list is hard coded. >>>>>>>>>> >>>>>>>>>> A possible solution to the extension problem is to change the >>>>>>>>>> code gen so that Augmentable gets a new method called >>>>>>>>>> getAllAugmentations >>>>>>>>>> which would return a list of all the attached augmentations. That >>>>>>>>>> way we >>>>>>>>>> could dynamically compare all matches, removing the need for a hard >>>>>>>>>> coded >>>>>>>>>> list of possible extension augmentations. This is a relatively >>>>>>>>>> simple fix, >>>>>>>>>> though (as Sam pointed out) the mdsal project is post code freeze so >>>>>>>>>> it >>>>>>>>>> would take a little maneuvering. >>>>>>>>>> >>>>>>>>>> Another issue with this solution, though this can be solved >>>>>>>>>> relatively easily is that the current implementation relies on the >>>>>>>>>> ordering >>>>>>>>>> of the Matches which I'm not sure is safe. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Decisions >>>>>>>>>> ======================== >>>>>>>>>> 1. We could fall back on the HE solution >>>>>>>>>> 2. We could continue with my patch which is imperfect but seems >>>>>>>>>> to cover more cases than the He plugin solution (though less "tried >>>>>>>>>> and >>>>>>>>>> true" as He was running for quite a while) >>>>>>>>>> 3. Take the Augmentable.getAllAugmentations route. >>>>>>>>>> 4. ?? >>>>>>>>>> >>>>>>>>> I think jozef mention that we should leave the comparison of >>>>>>>>> extension to the extensions itself, so extensions should implement >>>>>>>>> their >>>>>>>>> own hashcode/equals. Probably that's what you mean by (3)? >>>>>>>>> >>>>>>>>>> >>>>>>>>>> -J >>>>>>>>>> >>>>>>>>>> [1] https://bugs.opendaylight.org/show_bug.cgi?id=6146 >>>>>>>>>> [2] >>>>>>>>>> openflowplugin/applications/statistics-manager/src/main/java/org/opendaylight/openflowplugin/applications/statistics/manager/impl/helper/FlowComparatorFactory.java >>>>>>>>>> [3] https://git.opendaylight.org/gerrit/#/c/41542/ >>>>>>>>>> >>>>>>>>>> On Thu, Jul 21, 2016 at 4:20 AM, Abhijit Kumbhare < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>>> Hi folks, >>>>>>>>>>> >>>>>>>>>>> Getting this on the OpenFlow mailing list. Also changed the >>>>>>>>>>> subject to be slightly more related to the actual topic than >>>>>>>>>>> "Gerrit is >>>>>>>>>>> stuck" :) >>>>>>>>>>> >>>>>>>>>>> Anyway Josh - do you want to summarize your solution (and the >>>>>>>>>>> problem) for the folks new to this discussion? >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> Abhijit >>>>>>>>>>> >>>>>>>>>>> On Tue, Jul 19, 2016 at 4:19 PM, Josh Hershberg < >>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>> >>>>>>>>>>>> Which I think my patch is! The differences between the Match >>>>>>>>>>>> objects in question are syntactic and not semantic. I added a >>>>>>>>>>>> compare >>>>>>>>>>>> method that ignores the structural differences. >>>>>>>>>>>> >>>>>>>>>>>> On Tue, Jul 19, 2016 at 12:45 PM, Sam Hague <[email protected]> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> As Josh mentions the issue is that the match object coming >>>>>>>>>>>>> back is not the same as what was written to mdsal so it ends up >>>>>>>>>>>>> as a >>>>>>>>>>>>> different flow. We need a better way to compare the flows. >>>>>>>>>>>>> >>>>>>>>>>>>> On Jul 19, 2016 2:43 AM, "Josh Hershberg" <[email protected]> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Adding Sam. >>>>>>>>>>>>> >>>>>>>>>>>>> Sam, this is the alien flow issue. >>>>>>>>>>>>> >>>>>>>>>>>>> On Tue, Jul 19, 2016 at 5:35 AM, Anil Vishnoi < >>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> +adding other folks >>>>>>>>>>>>>> >>>>>>>>>>>>>> Hi Josh, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks for reminding me of the patch. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Andrej/Jozef, This is the patch, regarding which i asked a >>>>>>>>>>>>>> question on how we currently compare the operational flow and >>>>>>>>>>>>>> config flow >>>>>>>>>>>>>> in lithium plugin? >>>>>>>>>>>>>> >>>>>>>>>>>>>> In the helium plugin we have custom comparators to compare >>>>>>>>>>>>>> the config and operational flow that do explicit match using the >>>>>>>>>>>>>> matching >>>>>>>>>>>>>> construct of the flow ( + cookie). Looks like in lithium plugin >>>>>>>>>>>>>> we are >>>>>>>>>>>>>> using hashcode/equals for the same ( i didn't get a chance to >>>>>>>>>>>>>> look at the >>>>>>>>>>>>>> code yet), which i am afraid won't work for all the scenario, >>>>>>>>>>>>>> specially >>>>>>>>>>>>>> augmentation. Using custom comparator for matching flow for >>>>>>>>>>>>>> augmentation is >>>>>>>>>>>>>> also not really a good idea, because there can be many extension >>>>>>>>>>>>>> (openflow >>>>>>>>>>>>>> and vendor specific) and handling them in the plugin is not a >>>>>>>>>>>>>> good idea. In >>>>>>>>>>>>>> helium plugin, i added cookie as a part of the flow comparison, >>>>>>>>>>>>>> so if you >>>>>>>>>>>>>> have two flows that is different only by the extension matches, >>>>>>>>>>>>>> then user >>>>>>>>>>>>>> can use cookie to differentiate the flows, so custom comparator >>>>>>>>>>>>>> don't have >>>>>>>>>>>>>> to match based on the augmentation. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Can you please share some more details on the existing >>>>>>>>>>>>>> comparison mechanism and lets discuss how we can resolve this >>>>>>>>>>>>>> issue in >>>>>>>>>>>>>> better way. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks >>>>>>>>>>>>>> Anil >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Sat, Jul 16, 2016 at 9:49 PM, Josh Hershberg < >>>>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hi Anil, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Could you please take a look at this gerrit? Just want to >>>>>>>>>>>>>>> make sure it's not buried or stuck or something. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> thanks, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Josh >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> https://git.opendaylight.org/gerrit/#/c/41542/ >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> -- >>>>>>>>>>>>>> Thanks >>>>>>>>>>>>>> Anil >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Thanks >>>>>>>>> Anil >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Thanks >>>>>>>> Anil >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Thanks >>>>>>> Anil >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Thanks >>>>> Anil >>>>> >>>> >>>> >>> >>> >>> -- >>> Thanks >>> Anil >>> >> >> > > > -- > Thanks > Anil >
_______________________________________________ openflowplugin-dev mailing list [email protected] https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev
