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_*. 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"?
Regards,
Robert
signature.asc
Description: OpenPGP digital signature
_______________________________________________ openflowplugin-dev mailing list [email protected] https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev
