Operating System: All
Platform: All

Could you please point to the model which triggered this and also binary data you were trying to set?

The module is odl-lisp-proto.yang, lines 24-51, see my initial report for a link to gitweb. The binary data we tried to set was an byte[] taken from am InetAddress. The problem happens not when trying to set it, but when it has to be converted to text with .toString().

From the error log, may be you use Ipv4AddressBinary.getDefaultInstance("XXXX") to set binary data, and the "XXXX" contains '[', so you have the error.

Any updates on this? Would really appreciate a fix.

Thanks,
Vina

(In reply to Tony Tkacik from comment #1)
> Could you please point to the model which triggered this and also binary
> data you were trying to set?

On line 42 of the model that I linked to in the original bug report (odl-lisp-proto.yang) I'm defining a union of two binary types ipv4-address-binary and ipv6-address-binary as ip-address-binary. Yangtools autogenerates the IpAddressBinaryBuilder.java file under src/main/java instead of generated-sources/mdsal-binding, because it needs to be implemented. The implementation looks like this:

https://git.opendaylight.org/gerrit/gitweb?p=lispflowmapping.git;a=blob;f=mappingservice/lisp-proto/src/main/java/org/opendaylight/yang/gen/v1/urn/opendaylight/lfm/lisp/proto/rev151105/IpAddressBinaryBuilder.java;h=810d6cf7be2048e17ff0fd5d52be53817a23f906;hb=HEAD

Robert gave a quick look at the implementation (at the design forum), and said it should work as-is. However, if I try a IpAddressBinaryBuilder.getDefaultInstance("127.0.0.1") for example, I will get an exception.

The exception from the original bug report for example is trying to log a packet address, where the address is stored in an Ipv4AddressBinary type.

Some additional info, I retested things, since the original report was a while ago. When I add an Ipv4AddressBinary object to the datastore and read it back with RESTCONF, it is represented like this: "fwAAAQ==", which is the expected base64 encoding of the binary data (127.0.0.1). However, an IpAddressBinary object, which is the union, looks like this: "[B@6fadb49c" which looks an awful lot like a reference instead of the object inside. Hope this helps fix the bug quicker.

fix:
master - https://git.opendaylight.org/gerrit/#/c/38272/
Be - https://git.opendaylight.org/gerrit/#/c/38358/

(In reply to Peter Kajsa from comment #7)
> fix:
> master - https://git.opendaylight.org/gerrit/#/c/38272/
> Be - https://git.opendaylight.org/gerrit/#/c/38358/

I just tried the suggested fix. I still see "[B@125afdf3" for an ip-address-binary based field in RESTCONF output. The _value member in the IpAddressBinary object however looks ok. Here's some code I used to test the logging output, after reading an IPv4 address from a packet:

if (sourceRloc instanceof Inet4Address) {
Ipv4AddressBinary ipv4 = new Ipv4AddressBinary(sourceRloc.getAddress());
IpAddressBinary ip = new IpAddressBinary(ipv4);
LOG.info("Ipv4AddressBinary: {} | IpAddressBinary: {}", ipv4, ip);
}

The output looks like this:

2016-05-04 12:55:09,426 | INFO | lisp-sb-6-1 | MapRegisterSerializer | 255 - org.opendaylight.lispflowmapping.mappingservice.lisp-proto - 1.4.0.SNAPSHOT | Ipv4AddressBinary: Ipv4AddressBinary [_value=[127, 0, 0, 1]] | IpAddressBinary: IpAddressBinary [_ipv4AddressBinary=Ipv4AddressBinary [_value=[127, 0, 0, 1]], _value=[f, w, A, A, A, Q, =, =]]

RESTCONF output still shows the memory reference.

I just pushed some code that I use for RESTCONF testing here:
https://github.com/ljakab/lispflowmapping/tree/ip-address-binary

Please let us know, whether the exception from your first comment (java.lang.IllegalStateException: Could not construct instance ...) still persists ? I think it should be ok now.

However, there could be the same problem somewhere in Restconf as well, so we will look at it.

(In reply to Peter Kajsa from comment #9)
> Please let us know, whether the exception from your first comment
> (java.lang.IllegalStateException: Could not construct instance ...) still
> persists ? I think it should be ok now.

I can't reproduce that exception anymore, so I think it's ok.

>
> However, there could be the same problem somewhere in Restconf as well, so
> we will look at it.

Thanks!

(In reply to Lori Jakab from comment #10)
> (In reply to Peter Kajsa from comment #9)
> > Please let us know, whether the exception from your first comment
> > (java.lang.IllegalStateException: Could not construct instance ...) still
> > persists ? I think it should be ok now.
>
> I can't reproduce that exception anymore, so I think it's ok.

Actually, now that I tried to build some code using the union, I still see issues regardless of RESTCONF. You can take a look at the unit tests from the mappingservice.implementation module after applying this patch https://git.opendaylight.org/gerrit/#/c/38496/

LispMappingServiceTest.onAddMappingTest_noTransportAddress:196
Argument(s) are different! Wanted:
lispSB.sendMapNotify(
SendMapNotifyInput [_mapNotify=MapNotify [_keyId=1, augmentation=[]], _transportAddress=TransportAddress [_ipAddress=IpAddressBinary [_ipv4AddressBinary=Ipv4AddressBinary [_value=[1, 2, 3, 0]], _value=[[, B, @, 4, d, 0, 4, 0, 2, b]], _port=PortNumber [_value=4342], augmentation=[]], augmentation=[]]
);
-> at org.opendaylight.lispflowmapping.implementation.LispMappingServiceTest.onAddMappingTest_noTransportAddress(LispMappingServiceTest.java:196)
Actual invocation has different arguments:
lispSB.sendMapNotify(
SendMapNotifyInput [_mapNotify=MapNotify [_keyId=1, augmentation=[]], _transportAddress=TransportAddress [_ipAddress=IpAddressBinary [_ipv4AddressBinary=Ipv4AddressBinary [_value=[1, 2, 3, 0]], _value=[[, B, @, 1, 6, 0, c, 3, e, c, 1]], _port=PortNumber [_value=4342], augmentation=[]], augmentation=[]]

If you take a look at the IpAddressBinary _value, it still shows a memory reference instead of a base64 string.

(In reply to Lori Jakab from comment #11)
> If you take a look at the IpAddressBinary _value, it still shows a memory
> reference instead of a base64 string.

Or the bytes themselves, as it's shown for Ipv4AddressBinary _value.

Yangtools union codec fix: https://git.opendaylight.org/gerrit/#/c/38669/

Please retest with the both patches and let us know the result. Thanks.
MD-SAL: https://git.opendaylight.org/gerrit/#/c/38272/
Yangtools: https://git.opendaylight.org/gerrit/#/c/38669/

The two patches together fix all issues, including correct encoding in RESTCONF output. Thanks!

mdsal Be: https://git.opendaylight.org/gerrit/38358

yangtools need a manual cherry-pick.

Yangtools Be: https://git.opendaylight.org/gerrit/#/c/38853/1