On Fri, Apr 20, 2018 at 12:37 AM, Robert Varga <[email protected]> wrote: > On 19/04/18 21:49, Anil Vishnoi wrote: > > It is not a matter of 'it could be supported', anything not defying > laws > > of Physics can be supported. The question is: does it make sense? can > > you show an example when it would be immediately useful? > > > > That's where i brought the point previously that mdsal should not care > > how it can be used or what is the usecase. They should adhere to the > > defined types in ietf-types. > > This is where it gets tricky and relates to > https://git.opendaylight.org/gerrit/#/c/71072/. > > In order to provide better ergonomics and performance, there are things > in MD-SAL, which are providing model-specific hacks in an uniform way. > > One of these is IetfInetUtil, which is *not* MD-SAL core, but rather > model-specific support. You will note this class lives in three places: > a generic one, a 2010-09-24 and a 2013-07-15 one. These are classes > hand-coded by yours truly. > > OFP is using these utility methods, i.e. it is binding to a hand-written > MD-SAL utility method. > > The implementation of that class has changed to return > Ipv6AddressNoZone, which is a valid subclass of Ipv6Address -- as per > mapping we previously discussed. > > Where things broke is that the returned object no longer equals to > Ipv4Address(<same_string>), which is what OFP and others assumed. This > is AbstractIetfInetUtil.ipv4AddressFor(). > > The API contract of that method says: > > > /** > > * Create an Ipv4Address by interpreting input bytes as an IPv4 > address. > > * > > * @param bytes 4-byte array > > * @return An Ipv4Address object > > * @throws IllegalArgumentException if bytes has length different > from 4 > > * @throws NullPointerException if bytes is null > > */ > > You assume 'Create an Ipv4Address' to literally mean 'Creates an object > whose class is Ipv4Address' -- i.e. excluding subclasses. That is a > documentation gap, but I don't know how to improve the text. Patches for > clarification are always welcome. > > I will posit that the text therein can also be interpreted to include > Ipv4AddressNoZone -- i.e. a subclass of Ipv4Address, which does not > define new API elements and only guarantees that the string contained > within is a strict subset of what Ipv4Address can legally hold. > > The input into this method is an Inet4Address, which has no idea of what > a zone is, i.e. the caller has already lost any information about zone > it may have had -- which is an important point in a formal proof, if you > are into that kind of thing. > > The only thing that https://git.opendaylight.org/gerrit/#/c/71072/ does > is changing the implementation class, i.e. re-wrapping the string > contained in the returned Ipv4AddressNoZone through MD-SAL provided > guarantee that any Ipv4AddressNoZone string is a valid Ipv4Address string. > > > > As far as i understood, MDSAL can't implement it in a way I want > to use > > > (non-breakable), because of language limitation or some other > reasons. > > > So there are two options > > > > > > (1) We merge the change and move all the project to these API > (given > > > that they cannot be used in the way defined in yang model) > > > (2) Do not implement support for ipv4-adderss-no-zone (as it's > creating > > > confusion), and let user use the ipv4-address (which can support > no-zone > > > and zone ip addresses) > > > > (3) we change how generated DataObject.equals() works. > > > > Bye, > > Robert > > > > Unfortunately there is no one else in the community who can mediate > > here, it's just your words against my, so i would hold my argument here > > and leave it to community. Once a weather reports comes out about this > > change and community has no issue with it, i am fine with it. > > Thank you for your understanding. > > To drive the argument to conclusion, though, is the analysis why things > broke -- which is because Ipv4Address.equals() uses getClass() to > determine equality. Ipv4AddressNoZone inherits that -- it's not what > Ipv4AddressNoZone type or MD-SAL's treatment of it has done. It is what > Ipv4Address as an MD-SAL-generated type does. > I think you got my question finally. This is what basically broke the backward compatibility for ipv4address. derive type.
> > If we can make Ipv4Address.equals() final, we can define it in way, > which would prevent this sort of breakage happening again -- but that is: > 1) a restriction on how generated code can be used > 2) a relatively high-risk change (is there anybody manually subclassing > generated classes?) > 3) probably not a 5-line patch implementation > > The question to the community is: > > Is that what we want MD-SAL Binding V1 to do Fluorine onwards? > Weather report with these rationale would help. If community care, they will understand the issue and raise their concern if any, if not, it's mdsal committers decision. > > Regards, > Robert > > -- Thanks Anil
_______________________________________________ openflowplugin-dev mailing list [email protected] https://lists.opendaylight.org/mailman/listinfo/openflowplugin-dev
