From 8d18002e59e2b7e7fca95e52d2cb4a75d1023117 Mon Sep 17 00:00:00 2001
From: "miroslav.macko" <miroslav.macko@pantheon.tech>
Date: Wed, 14 Jun 2017 14:06:30 +0200
Subject: [PATCH] Fix port update

- Update yang models. Add grouping common port with mask.
- Use config mask in the PortConvertor and PortMessageSerializer.
- Update unit tests.

Resolves: bug 4747

Change-Id: If558fea34f86c026ca45ba2853107c2cd82c7f50
Signed-off-by: miroslav.macko <miroslav.macko@pantheon.tech>
---
 .../main/yang/opendaylight-port-types.yang    | 18 ++++++++++-------
 .../messages/PortMessageSerializer.java       | 14 ++++++++-----
 .../singlelayer/SingleLayerPortService.java   |  6 +++---
 .../messages/PortMessageSerializerTest.java   | 12 ++++++++++-
 .../md/core/sal/convertor/PortConvertor.java  | 20 ++++++++++++++++---
 .../core/sal/convertor/PortConvertorTest.java | 10 ++++++++--
 6 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/model/model-flow-base/src/main/yang/opendaylight-port-types.yang b/model/model-flow-base/src/main/yang/opendaylight-port-types.yang
index 9c6d3a34bd..fa6e1fcdfd 100644
--- a/model/model-flow-base/src/main/yang/opendaylight-port-types.yang
+++ b/model/model-flow-base/src/main/yang/opendaylight-port-types.yang
@@ -92,6 +92,15 @@ module opendaylight-port-types {
         }
     }
 
+    grouping  common-port-with-mask {
+        uses common-port;
+
+        leaf mask {
+            type port-config;
+            description "Bitmap of OFPPC-* flags to be changed";
+        }
+    }
+
     grouping flow-port-status {
         leaf reason {
             type port-reason;
@@ -159,12 +168,7 @@ module opendaylight-port-types {
                     type uint32;
                 }
 
-                uses common-port;
-
-                leaf mask {
-                    type port-config;
-                    description "Bitmap of OFPPC-* flags to be changed";
-                }
+                uses common-port-with-mask;
 
                 leaf container-name {
                     type string;
@@ -182,7 +186,7 @@ module opendaylight-port-types {
     }
 
     container port-message {
-        uses common-port;
+        uses common-port-with-mask;
         uses ofproto:ofHeader;
     }
 
diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializer.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializer.java
index 9f4b782286..c11f8b3b10 100644
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializer.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializer.java
@@ -8,8 +8,10 @@
 
 package org.opendaylight.openflowplugin.impl.protocol.serialization.messages;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableMap;
 import io.netty.buffer.ByteBuf;
+import java.util.Objects;
 import org.opendaylight.openflowjava.util.ByteBufUtils;
 import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion;
 import org.opendaylight.openflowplugin.openflow.md.util.OpenflowPortsUtil;
@@ -23,9 +25,12 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.P
  * OF protocol versions: 1.3.
  */
 public class PortMessageSerializer extends AbstractMessageSerializer<PortMessage> {
+
     private static final byte PADDING_IN_PORT_MOD_MESSAGE_01 = 4;
     private static final byte PADDING_IN_PORT_MOD_MESSAGE_02 = 2;
     private static final byte PADDING_IN_PORT_MOD_MESSAGE_03 = 4;
+    private static final int DEFAULT_PORT_CONFIG_MASK = createPortConfigBitMask(
+            new PortConfig(true, true, true, true));
 
     @Override
     public void serialize(final PortMessage message, final ByteBuf outBuffer) {
@@ -35,9 +40,8 @@ public class PortMessageSerializer extends AbstractMessageSerializer<PortMessage
         outBuffer.writeZero(PADDING_IN_PORT_MOD_MESSAGE_01);
         outBuffer.writeBytes(IetfYangUtil.INSTANCE.bytesFor(message.getHardwareAddress()));
         outBuffer.writeZero(PADDING_IN_PORT_MOD_MESSAGE_02);
-        final int portConfigBitMask = createPortConfigBitMask(message.getConfiguration());
-        outBuffer.writeInt(portConfigBitMask); // Configuration
-        outBuffer.writeInt(portConfigBitMask); // Configuration mask
+        outBuffer.writeInt(createPortConfigBitMask(message.getConfiguration()));
+        outBuffer.writeInt(MoreObjects.firstNonNull(createPortConfigBitMask(message.getMask()), DEFAULT_PORT_CONFIG_MASK));
         outBuffer.writeInt(createPortFeaturesBitMask(message.getAdvertisedFeatures()));
         outBuffer.writeZero(PADDING_IN_PORT_MOD_MESSAGE_03);
         outBuffer.setShort(index + 2, outBuffer.writerIndex() - index);
@@ -48,8 +52,8 @@ public class PortMessageSerializer extends AbstractMessageSerializer<PortMessage
         return 16;
     }
 
-    private static int createPortConfigBitMask(final PortConfig config) {
-        return ByteBufUtils.fillBitMaskFromMap(ImmutableMap
+    private static Integer createPortConfigBitMask(final PortConfig config) {
+        return Objects.isNull(config) ? null : ByteBufUtils.fillBitMaskFromMap(ImmutableMap
                 .<Integer, Boolean>builder()
                 .put(0, config.isPORTDOWN())
                 .put(2, config.isNORECV())
diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/singlelayer/SingleLayerPortService.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/singlelayer/SingleLayerPortService.java
index 814e5a0304..34aa509522 100644
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/singlelayer/SingleLayerPortService.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/services/singlelayer/SingleLayerPortService.java
@@ -13,12 +13,12 @@ import org.opendaylight.openflowplugin.api.openflow.device.RequestContextStack;
 import org.opendaylight.openflowplugin.api.openflow.device.Xid;
 import org.opendaylight.openflowplugin.impl.services.AbstractSimpleService;
 import org.opendaylight.openflowplugin.impl.services.util.ServiceException;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.CommonPort;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.CommonPortWithMask;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.PortMessageBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.OfHeader;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 
-public final class SingleLayerPortService<O extends DataObject> extends AbstractSimpleService<CommonPort, O> {
+public final class SingleLayerPortService<O extends DataObject> extends AbstractSimpleService<CommonPortWithMask, O> {
 
     public SingleLayerPortService(
         final RequestContextStack requestContextStack,
@@ -28,7 +28,7 @@ public final class SingleLayerPortService<O extends DataObject> extends Abstract
     }
 
     @Override
-    protected OfHeader buildRequest(final Xid xid, final CommonPort input) throws ServiceException {
+    protected OfHeader buildRequest(final Xid xid, final CommonPortWithMask input) throws ServiceException {
         return new PortMessageBuilder(input)
                 .setVersion(getVersion())
                 .setXid(xid.getValue())
diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializerTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializerTest.java
index a2ef50fb21..c3bcd5bdd2 100644
--- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializerTest.java
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/protocol/serialization/messages/PortMessageSerializerTest.java
@@ -69,6 +69,7 @@ public class PortMessageSerializerTest extends AbstractSerializerTest {
             .setVersion(VERSION)
             .setPortNumber(new PortNumberUni(PORT_NUMBER))
             .setConfiguration(new PortConfig(IS_NOFWD, IS_NOPACKETIN, IS_NORECV, IS_PORTDOWN))
+            .setMask(new PortConfig(true, true, true, true))
             .setAdvertisedFeatures(new PortFeatures(
                     IS_AUTOENG,
                     IS_COPPER,
@@ -125,8 +126,17 @@ public class PortMessageSerializerTest extends AbstractSerializerTest {
                 .put(5, IS_NOFWD)
                 .put(6, IS_NOPACKETIN)
                 .build());
+
+        final int mask = ByteBufUtils.fillBitMaskFromMap(ImmutableMap
+                .<Integer, Boolean>builder()
+                .put(0, true)
+                .put(2, true)
+                .put(5, true)
+                .put(6, true)
+                .build());
+
         assertEquals(out.readInt(), config);
-        assertEquals(out.readInt(), config);
+        assertEquals(out.readInt(), mask);
 
         // Port features
         assertEquals(out.readInt(), ByteBufUtils.fillBitMask(0,
diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertor.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertor.java
index bd27e21264..2ca332aef3 100644
--- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertor.java
+++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertor.java
@@ -9,14 +9,16 @@
 package org.opendaylight.openflowplugin.openflow.md.core.sal.convertor;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.MoreObjects;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Objects;
 import java.util.Set;
 import org.opendaylight.openflowplugin.api.openflow.md.util.OpenflowVersion;
-import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
 import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.common.Convertor;
 import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.VersionConvertorData;
 import org.opendaylight.openflowplugin.openflow.md.util.OpenflowPortsUtil;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.port.mod.port.Port;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortConfig;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortConfigV10;
@@ -60,6 +62,11 @@ public class PortConvertor extends Convertor<Port, PortModInput, VersionConverto
 
     private static PortConfig maskPortConfigFields(
             org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.PortConfig configData) {
+
+        if (Objects.isNull(configData)) {
+            return null;
+        }
+
         Boolean portDown = configData.isPORTDOWN();
         Boolean noRecv = configData.isNORECV();
         Boolean noFwd = configData.isNOFWD();
@@ -71,6 +78,11 @@ public class PortConvertor extends Convertor<Port, PortModInput, VersionConverto
 
     private static PortConfigV10 maskPortConfigV10Fields(
             org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.PortConfig configData) {
+
+        if (Objects.isNull(configData)) {
+            return null;
+        }
+
         Boolean portDown = configData.isPORTDOWN();
         Boolean noRecv = configData.isNORECV();
         Boolean noFwd = configData.isNOFWD();
@@ -165,14 +177,16 @@ public class PortConvertor extends Convertor<Port, PortModInput, VersionConverto
                 OpenflowPortsUtil.getProtocolPortNumber(OpenflowVersion.get(data.getVersion()), source.getPortNumber())));
 
         portModInputBuilder.setConfig(config);
-        portModInputBuilder.setMask(config);
+        portModInputBuilder.setMask(MoreObjects.firstNonNull(maskPortConfigFields(source.getMask()),
+                new PortConfig(true, true, true, true)));
 
         portModInputBuilder.setHwAddress(new MacAddress(source.getHardwareAddress()));
 
         portModInputBuilder.setVersion(data.getVersion());
 
         portModInputBuilder.setConfigV10(configV10);
-        portModInputBuilder.setMaskV10(configV10);
+        portModInputBuilder.setMaskV10(MoreObjects.firstNonNull(maskPortConfigV10Fields(source.getMask()),
+                new PortConfigV10(true, true, true, true, true, true, true)));
         portModInputBuilder.setAdvertiseV10(getPortFeaturesV10(source.getAdvertisedFeatures()));
         return portModInputBuilder.build();
     }
diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertorTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertorTest.java
index 98a3adaf94..06e143c648 100644
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertorTest.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/PortConvertorTest.java
@@ -47,6 +47,12 @@ public class PortConvertorTest {
             new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortConfig(
                     false, false, false, false);
 
+    private org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortConfig configMask31 =
+            new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortConfig(
+            true, true, true, true);
+
+    private PortConfigV10 portConfMaskV10 = new PortConfigV10(true, true, true, true, true, true, true);;
+
     /**
      * test of {@link org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.PortConvertor#convert(org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.port.rev130925.port.mod.port.Port, org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.VersionConvertorData)} }
      */
@@ -70,14 +76,14 @@ public class PortConvertorTest {
         PortModInputBuilder portModInputBld = new PortModInputBuilder();
 
         portModInputBld.setConfig(config31);
-        portModInputBld.setMask(config31);
+        portModInputBld.setMask(configMask31);
         portModInputBld.setPortNo(
                 new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.PortNumber(42L));
         portModInputBld.setHwAddress(new MacAddress(DEFAULT_MAC_ADDRESS));
         portModInputBld.setAdvertise(portf31);
 
         portModInputBld.setConfigV10(portConfV10);
-        portModInputBld.setMaskV10(portConfV10);
+        portModInputBld.setMaskV10(portConfMaskV10);
         portModInputBld.setAdvertiseV10(
                 new PortFeaturesV10(null, null, null, null, null, null, null, true, null, null, null, null));
 
-- 
2.36.6