On Thu, Apr 19, 2018 at 2:17 AM, Robert Varga <[email protected]> wrote: > On 18/04/18 23:18, Anil Vishnoi wrote: > > Robert, please respond to this thread, so others also get some clarity > > on it, so you don't have to answer all the projects ;) > > > > On Wed, Apr 18, 2018 at 2:02 PM, Anil Vishnoi <[email protected] > > <mailto:[email protected]>> wrote: > > > > > > > > On Wed, Apr 18, 2018 at 1:13 PM, Robert Varga <[email protected] > > <mailto:[email protected]>> wrote: > > > > On 18/04/18 21:17, Anil Vishnoi wrote: > > > > > > > > > On Wed, Apr 18, 2018 at 10:31 AM, Robert Varga <[email protected] > <mailto:[email protected]> > > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > > > Object.equals() is required to be reflexive. Your > statement "this is > > > not correct behavior" is based on your human understanding > of the > > > model (most notably reading English text in description), > not > > > something automation can infer. Therefore codegen cannot > generate an > > > .equals() method, which would be both conforming to the > API contract > > > and supporting "correct behavior". > > > > > > > Hello Anil, > > > > > Sorry robert, but i am not able to make sense out of what you > are > > > saying. English language can be interpreted in many ways, but > hopefully > > > you will agree that patterns has clear interpretation. Both > ipv4-address > > > and ipv4-address-no-zone has clear pattern defined. > > > > > > typedef ipv4-address-no-zone { > > > type ipv4-address { > > ^^^^^^^^^^^^^^^^^ > > > > > pattern '[0-9\.]*'; > > > } > > > } > > > > > > > > > typedef ipv4-prefix { > > > type string { > > > pattern > > > '(([0-9]|[1-9][0-9]|1[0-9][0- > 9]|2[0-4][0-9]|25[0-5])\.){3}' > > > + '([0-9]|[1-9][0-9]|1[0-9][0-9] > |2[0-4][0-9]|25[0-5])' > > > + '/(([0-9])|([1-2][0-9])|(3[0-2]))'; > > > } > > > } > > > > Wrong type, you meant to quote this: > > > > > typedef ipv4-address { > > > type string { > > > pattern > > > '(([0-9]|[1-9][0-9]|1[0-9][0- > 9]|2[0-4][0-9]|25[0-5])\.){3}' > > > + '([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])' > > > + '(%[\p{N}\p{L}]+)?'; > > > } > > > > but aside from that, yes those patterns are clear. What is clear > to > > humans, but not machines, is that: > > > > 1) an ipv4-address string really has two parts, a dotted-quad > and an > > optional zone prefixed with % > > > > 2) ipv4-address-no-zone is an ipv4-address, which does not have > > a zone > > > > Arriving at this distinction by analyzing the patterns involved > > is the > > challenge here -- as those are the only bits which a > > machine-understandable. I am pretty sure an algorithm to solve > that > > problem, if it is even possible, would run in non-linear time > > and would > > probably be a major breakthrough in information technology. > > > > Given that these pattern are already mapped to certain derive type, > > i assuming you don't have to do any breakthrough here to > > differentiate ipv4-address and ipv4-address-no-address, but i think > > the problem here is the way it's derived in the yang model itself. > > > > > > > And apart from that the failure is happening because equals is > expecting > > > different class then ipv4address (which is changed in the > above gerrit). > > > org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang. > ietf.inet.types.rev130715.Ipv4Address<Ipv4Address{_value=0.1.2.3}> > > > but was: > > > org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang. > ietf.inet.types.rev130715.Ipv4AddressNoZone<Ipv4Address{_value=0.1.2.3}> > > > > > > > > > I believe this is most probably happening because the way > mdsal is > > > generating the code now for no-zone in your latest patch. > > > > > > Currently generated code for ipv4-address-no-zone is extending > > > ipv4-addresss (which is weird to me , subset extending > superset) > > > > You have to understand how YANG type system works, which is > > detailed in > > RFC7950 sections 4.2.4, 4.2.5, 7.3 and 7.4. > > > > TL;DR; of that is that all YANG types are derived from a set of > > fixed > > built-in types. Derivation works by placing additional > > restrictions on > > the data, not extending the data set -- which is where the > > weirdness you > > perceive comes from. > > > > In in any case > > > > > public class Ipv4AddressNoZone extends Ipv4Address > > > > is an accurate mapping of the semantic relationship between the > > two types: > > > > - > > > > any valid Ipv4AddressNoZone is a valid Ipv4Address, but not > > vice-versa. > > - creating an Ipv4Address from an Ipv4AddressNoZone is dirt cheap > > - creating an Ipv4AddressNoZone from an Ipv4Address requires > > validation > > > > It is therefore clear that IetfInetUtil should be handing out > > Ipv4AddressNoZone objects -- as that is the semantically correct > > type > > capture of what is inside an Inet4Address.getAddress() array. > > > > > Given that both ipv4-address-no-zone and ipv4-address are > different > > > construct by definition, if mdsal generates these construct > separately > > > (without extending) that should keep the backward > compatibility without > > > breaking any downstream code. > > > > They are not completely different constructs, they are related > > through > > derivation: > > - string is the base type of ipv4-address > > - ipv4-address is a type derived from string > > - ipv4-address-no-zone is a type derived from ipv4-address > > > > Okay understood, it also means application can use these two derive > > types individually and should be comparable ? If that is the case, > > why the equal is failing in below line, given that match.getNwSrc() > > is returning IPv4Address? > > it is *declared* to return Ipv4Address, but returns a subclass. > > > Assert.assertEquals("Wrong nw-src", new > > Ipv4Address("16.17.18.19"), match.getNwSrc()); > > > > > > I believe it's because internally for all the ip-address type it's > > generating IPv4AddressNoZone object? Also why i have to change my > > yang model to not use ipv4-address? > > > > https://git.opendaylight.org/gerrit/#/c/71083/3/extension/ > openflowjava-extension-nicira/src/main/yang/nicira-action.yang > > <https://git.opendaylight.org/gerrit/#/c/71083/3/extension/ > openflowjava-extension-nicira/src/main/yang/nicira-action.yang> > > You do not have to, but... > > > This is kind of causing an issue here in the overall understanding > > of using ipv4-address and ipv4-address-no-zone. There are two things > > that you mentioned > > (1) This is not conforming to the API contract but it's "correct" > > behavior. > > (2) > > > > any valid Ipv4AddressNoZone is a valid Ipv4Address, but not > vice-versa. > > > > (1) is going to introduce a confusion for the user here because of > > the way mdsal handles it and reading (2) basically says you don't > > really need ipv4-address-no-zone, you can use ipv4-address for > > ipv4-address-no-zone as well. > > > > Based on these data points, i believe we should better not support > > ipv4-address-no-zone and suggest user to use ipv4-address, rather > > then introducing this feature and confusing users on how they should > > write the code around it. > > ... it is matter of what is the correct model. Given that OpenFlow has > no notion of a zone, I believe ipv4-address-no-zone is the correct type > representing OFPXMT_OFB_IPV4_*.
Please do not see it from specific project perspective, neither a code generator should care what is really used and what is not. They go by what is defined in yang model and generate clean interfaces. > To illustrate, can you explain what > happens when a user specifies two matches, which differ only in zone, > for example "1.2.3.4%a" and "1.2.3.4%b"? > OpenFlow plugin should fail, if it's not supporting zone, but user can always write an application who uses zone. I think it's none of mdsal concern on how consumer is using these data type, neither mdsal should be influence by the context of consumption model when it comes to implementing defined data types. > > Regards, > Robert > > -- Thanks Anil
_______________________________________________ openflowplugin-dev mailing list [email protected] https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev
