Bug fixes of calculating message length for InfoRequest/Reply
Changes
1. Wrap added InfoReqest processing with try..finally statement
2. InfoRequest serialize bug fix
3. Overide WriteTo() methods of DefaultLispInfoRequest
and DefaultLispReply
Change-Id: Ifc74619508a004f3fa7c940c30a5905d2cd27963
diff --git a/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispChannelHandler.java b/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispChannelHandler.java
index 1041370..5f809bf 100644
--- a/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispChannelHandler.java
+++ b/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispChannelHandler.java
@@ -63,19 +63,17 @@
ctx.writeAndFlush(mapNotify);
}
+
+ if (msg instanceof LispInfoRequest) {
+ LispMapServer mapServer = new LispMapServer();
+ LispInfoReply infoReply = mapServer.processInfoRequest((LispInfoRequest) msg);
+
+ ctx.writeAndFlush(infoReply);
+ }
} finally {
+ // try to remove the received message form the buffer
ReferenceCountUtil.release(msg);
}
-
- if (msg instanceof LispInfoRequest) {
- LispMapServer mapServer = new LispMapServer();
- LispInfoReply infoReply = mapServer.processInfoRequest((LispInfoRequest) msg);
-
- // try to remove the received info-request message from buffer
- ReferenceCountUtil.release(msg);
-
- ctx.writeAndFlush(infoReply);
- }
}
@Override
diff --git a/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispMapServer.java b/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispMapServer.java
index 4e555ee..f664c4c 100644
--- a/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispMapServer.java
+++ b/protocols/lisp/ctl/src/main/java/org/onosproject/lisp/ctl/LispMapServer.java
@@ -199,7 +199,7 @@
requestBuilder.withIsInfoReply(request.isInfoReply());
requestBuilder.withMaskLength(request.getMaskLength());
- LispInfoRequest authRequest = requestBuilder.build();
+ LispInfoRequest authRequest = requestBuilder.build();
return Arrays.equals(authRequest.getAuthData(), request.getAuthData());
}
diff --git a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReply.java b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReply.java
index 5ab4f96..dae9e10 100644
--- a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReply.java
+++ b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReply.java
@@ -43,6 +43,12 @@
private final LispNatLcafAddress natLcafAddress;
+ static final InfoReplyWriter WRITER;
+
+ static {
+ WRITER = new InfoReplyWriter();
+ }
+
/**
* A private constructor that protects object instantiation from external.
*
@@ -69,6 +75,11 @@
}
@Override
+ public void writeTo(ByteBuf byteBuf) throws LispWriterException {
+ WRITER.writeTo(byteBuf, this);
+ }
+
+ @Override
public String toString() {
return toStringHelper(this)
.add("type", getType())
diff --git a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoRequest.java b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoRequest.java
index 18a055f..f0dbba4 100644
--- a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoRequest.java
+++ b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoRequest.java
@@ -24,6 +24,7 @@
import org.onosproject.lisp.msg.exceptions.LispReaderException;
import org.onosproject.lisp.msg.exceptions.LispWriterException;
import org.onosproject.lisp.msg.types.LispAfiAddress;
+import org.onosproject.lisp.msg.types.LispNoAddress;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -35,9 +36,17 @@
/**
* Default LISP info request message class.
*/
-public class DefaultLispInfoRequest extends DefaultLispInfo implements LispInfoRequest {
+public class DefaultLispInfoRequest extends DefaultLispInfo
+ implements LispInfoRequest {
- private static final Logger log = LoggerFactory.getLogger(DefaultLispInfoRequest.class);
+ private static final Logger log =
+ LoggerFactory.getLogger(DefaultLispInfoRequest.class);
+
+ static final InfoRequestWriter WRITER;
+
+ static {
+ WRITER = new InfoRequestWriter();
+ }
/**
* A private constructor that protects object instantiation from external.
@@ -51,10 +60,13 @@
* @param maskLength EID prefix mask length
* @param eidPrefix EID prefix
*/
- protected DefaultLispInfoRequest(boolean infoReply, long nonce, short keyId, short authDataLength,
- byte[] authData, int ttl, byte maskLength,
+ protected DefaultLispInfoRequest(boolean infoReply, long nonce, short keyId,
+ short authDataLength, byte[] authData,
+ int ttl, byte maskLength,
LispAfiAddress eidPrefix) {
- super(infoReply, nonce, keyId, authDataLength, authData, ttl, maskLength, eidPrefix);
+
+ super(infoReply, nonce, keyId, authDataLength, authData, ttl,
+ maskLength, eidPrefix);
}
@Override
@@ -95,7 +107,13 @@
eidPrefix) + Arrays.hashCode(authData);
}
- public static final class DefaultInfoRequestBuilder implements InfoRequestBuilder {
+ @Override
+ public void writeTo(ByteBuf byteBuf) throws LispWriterException {
+ WRITER.writeTo(byteBuf, this);
+ }
+
+ public static final class DefaultInfoRequestBuilder
+ implements InfoRequestBuilder {
private boolean infoReply;
private long nonce;
@@ -174,17 +192,21 @@
// if authentication data is not specified, we will calculate it
if (authData == null) {
- LispAuthenticationFactory factory = LispAuthenticationFactory.getInstance();
+ LispAuthenticationFactory factory =
+ LispAuthenticationFactory.getInstance();
- authDataLength = LispAuthenticationKeyEnum.valueOf(keyId).getHashLength();
+ authDataLength =
+ LispAuthenticationKeyEnum.valueOf(keyId).getHashLength();
byte[] tmpAuthData = new byte[authDataLength];
Arrays.fill(tmpAuthData, (byte) 0);
authData = tmpAuthData;
ByteBuf byteBuf = Unpooled.buffer();
try {
- new DefaultLispInfoRequest(infoReply, nonce, keyId, authDataLength,
- authData, ttl, maskLength, eidPrefix).writeTo(byteBuf);
+ new DefaultLispInfoRequest(infoReply, nonce, keyId,
+ authDataLength, authData, ttl,
+ maskLength, eidPrefix)
+ .writeTo(byteBuf);
} catch (LispWriterException e) {
log.warn("Failed to serialize info request", e);
}
@@ -196,7 +218,8 @@
log.warn("Must specify authentication key");
}
- authData = factory.createAuthenticationData(valueOf(keyId), authKey, bytes);
+ authData = factory
+ .createAuthenticationData(valueOf(keyId), authKey, bytes);
}
return new DefaultLispInfoRequest(infoReply, nonce, keyId,
@@ -207,10 +230,12 @@
/**
* A LISP message reader for InfoRequest message.
*/
- public static class InfoRequestReader implements LispMessageReader<LispInfoRequest> {
+ public static class InfoRequestReader
+ implements LispMessageReader<LispInfoRequest> {
@Override
- public LispInfoRequest readFrom(ByteBuf byteBuf) throws LispParseError, LispReaderException {
+ public LispInfoRequest readFrom(ByteBuf byteBuf)
+ throws LispParseError, LispReaderException {
LispInfo lispInfo = DefaultLispInfo.deserialize(byteBuf);
@@ -229,11 +254,19 @@
/**
* A LISP message writer for InfoRequest message.
*/
- public static final class InfoRequestWriter implements LispMessageWriter<LispInfoRequest> {
+ public static final class InfoRequestWriter
+ implements LispMessageWriter<LispInfoRequest> {
@Override
- public void writeTo(ByteBuf byteBuf, LispInfoRequest message) throws LispWriterException {
+ public void writeTo(ByteBuf byteBuf, LispInfoRequest message)
+ throws LispWriterException {
+
DefaultLispInfo.serialize(byteBuf, message);
+
+ //Fill AFI=0, no address
+ new LispAfiAddress.AfiAddressWriter()
+ .writeTo(byteBuf, new LispNoAddress());
+
}
}
}
diff --git a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispLcafAddress.java b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispLcafAddress.java
index 97e4752..93dfc65 100644
--- a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispLcafAddress.java
+++ b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispLcafAddress.java
@@ -240,7 +240,8 @@
* @param byteBuf netty byte buffer
*/
public static void updateLength(int lcafIndex, ByteBuf byteBuf) {
- byteBuf.setByte(lcafIndex + LENGTH_FIELD_INDEX, byteBuf.writerIndex() - COMMON_HEADER_SIZE);
+ byteBuf.setByte(lcafIndex + LENGTH_FIELD_INDEX,
+ byteBuf.writerIndex() - COMMON_HEADER_SIZE - lcafIndex);
}
/**
@@ -361,40 +362,48 @@
* @return LispLcafAddress instance
*/
public LispLcafAddress build() {
- return new LispLcafAddress(LispCanonicalAddressFormatEnum.valueOf(lcafType),
- reserved1, reserved2, flag, length);
+ return new LispLcafAddress(LispCanonicalAddressFormatEnum
+ .valueOf(lcafType),
+ reserved1, reserved2, flag, length);
}
}
/**
* LISP LCAF reader class.
*/
- public static class LcafAddressReader implements LispAddressReader<LispLcafAddress> {
+ public static class LcafAddressReader
+ implements LispAddressReader<LispLcafAddress> {
private static final int LCAF_TYPE_FIELD_INDEX = 4;
@Override
- public LispLcafAddress readFrom(ByteBuf byteBuf) throws LispParseError, LispReaderException {
+ public LispLcafAddress readFrom(ByteBuf byteBuf)
+ throws LispParseError, LispReaderException {
int index = byteBuf.readerIndex();
// LCAF type -> 8 bits
- byte lcafType = (byte) byteBuf.getUnsignedByte(index + LCAF_TYPE_FIELD_INDEX);
+ byte lcafType = (byte) byteBuf
+ .getUnsignedByte(index + LCAF_TYPE_FIELD_INDEX);
if (lcafType == APPLICATION_DATA.getLispCode()) {
- return new LispAppDataLcafAddress.AppDataLcafAddressReader().readFrom(byteBuf);
+ return new LispAppDataLcafAddress
+ .AppDataLcafAddressReader().readFrom(byteBuf);
}
if (lcafType == LIST.getLispCode()) {
- return new LispListLcafAddress.ListLcafAddressReader().readFrom(byteBuf);
+ return new LispListLcafAddress
+ .ListLcafAddressReader().readFrom(byteBuf);
}
if (lcafType == SEGMENT.getLispCode()) {
- return new LispSegmentLcafAddress.SegmentLcafAddressReader().readFrom(byteBuf);
+ return new LispSegmentLcafAddress
+ .SegmentLcafAddressReader().readFrom(byteBuf);
}
if (lcafType == SOURCE_DEST.getLispCode()) {
- return new LispSourceDestLcafAddress.SourceDestLcafAddressReader().readFrom(byteBuf);
+ return new LispSourceDestLcafAddress
+ .SourceDestLcafAddressReader().readFrom(byteBuf);
}
if (lcafType == TRAFFIC_ENGINEERING.getLispCode()) {
@@ -410,26 +419,28 @@
/**
* LISP LCAF address writer class.
*/
- public static class LcafAddressWriter implements LispAddressWriter<LispLcafAddress> {
+ public static class LcafAddressWriter
+ implements LispAddressWriter<LispLcafAddress> {
@Override
- public void writeTo(ByteBuf byteBuf, LispLcafAddress address) throws LispWriterException {
+ public void writeTo(ByteBuf byteBuf, LispLcafAddress address)
+ throws LispWriterException {
switch (address.getType()) {
case APPLICATION_DATA:
- new LispAppDataLcafAddress.AppDataLcafAddressWriter().writeTo(byteBuf,
- (LispAppDataLcafAddress) address);
+ new LispAppDataLcafAddress.AppDataLcafAddressWriter()
+ .writeTo(byteBuf, (LispAppDataLcafAddress) address);
break;
case LIST:
new LispListLcafAddress.ListLcafAddressWriter().writeTo(byteBuf,
(LispListLcafAddress) address);
break;
case SEGMENT:
- new LispSegmentLcafAddress.SegmentLcafAddressWriter().writeTo(byteBuf,
- (LispSegmentLcafAddress) address);
+ new LispSegmentLcafAddress.SegmentLcafAddressWriter()
+ .writeTo(byteBuf, (LispSegmentLcafAddress) address);
break;
case SOURCE_DEST:
- new LispSourceDestLcafAddress.SourceDestLcafAddressWriter().writeTo(byteBuf,
- (LispSourceDestLcafAddress) address);
+ new LispSourceDestLcafAddress.SourceDestLcafAddressWriter()
+ .writeTo(byteBuf, (LispSourceDestLcafAddress) address);
break;
case TRAFFIC_ENGINEERING:
new LispTeLcafAddress.TeLcafAddressWriter().writeTo(byteBuf,
diff --git a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispNatLcafAddress.java b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispNatLcafAddress.java
index 70f046a..9a1e504 100644
--- a/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispNatLcafAddress.java
+++ b/protocols/lisp/msg/src/main/java/org/onosproject/lisp/msg/types/LispNatLcafAddress.java
@@ -162,7 +162,8 @@
Objects.equals(this.etrUdpPortNumber, other.etrUdpPortNumber) &&
Objects.equals(this.globalEtrRlocAddress, other.globalEtrRlocAddress) &&
Objects.equals(this.msRlocAddress, other.msRlocAddress) &&
- Objects.equals(this.privateEtrRlocAddress, other.privateEtrRlocAddress);
+ Objects.equals(this.privateEtrRlocAddress, other.privateEtrRlocAddress) &&
+ Objects.equals(this.rtrRlocAddresses, other.rtrRlocAddresses);
}
return false;
}
@@ -175,6 +176,7 @@
.add("global ETR RLOC address", globalEtrRlocAddress)
.add("Map Server RLOC address", msRlocAddress)
.add("private ETR RLOC address", privateEtrRlocAddress)
+ .add("RTR RLOC addresses", rtrRlocAddresses)
.toString();
}
diff --git a/protocols/lisp/msg/src/test/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReplyTest.java b/protocols/lisp/msg/src/test/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReplyTest.java
index 6b68d54..6ff19a7 100644
--- a/protocols/lisp/msg/src/test/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReplyTest.java
+++ b/protocols/lisp/msg/src/test/java/org/onosproject/lisp/msg/protocols/DefaultLispInfoReplyTest.java
@@ -72,7 +72,7 @@
.withNonce(1L)
.withKeyId((short) 1)
.withAuthKey(AUTH_KEY)
- .withIsInfoReply(false)
+ .withIsInfoReply(true)
.withMaskLength((byte) 1)
.withEidPrefix(address1)
.withNatLcafAddress(natLcafAddress1).build();
@@ -83,7 +83,7 @@
.withNonce(1L)
.withKeyId((short) 1)
.withAuthKey(AUTH_KEY)
- .withIsInfoReply(false)
+ .withIsInfoReply(true)
.withMaskLength((byte) 1)
.withEidPrefix(address1)
.withNatLcafAddress(natLcafAddress1).build();
@@ -145,7 +145,7 @@
.withPrivateEtrRlocAddress(privateEtrRlocAddress1)
.build();
- assertThat(reply.isInfoReply(), is(false));
+ assertThat(reply.isInfoReply(), is(true));
assertThat(reply.getNonce(), is(1L));
assertThat(reply.getKeyId(), is((short) 1));
assertThat(reply.getMaskLength(), is((byte) 1));