From 46fcbe4b910fccda99f12a67b86770bf8e1c7a89 Mon Sep 17 00:00:00 2001 From: Jian Li Date: Thu, 13 Jun 2019 13:45:26 +0900 Subject: [PATCH] Remove duplicated ovsdb javadoc, harden setController logic Change-Id: I1b0aad513ded71a767d72e6e2eafb836366285c5 --- .../ovsdb/controller/OvsdbClientService.java | 37 +++++----- .../controller/driver/DefaultOvsdbClient.java | 68 ++++--------------- 2 files changed, 34 insertions(+), 71 deletions(-) diff --git a/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/OvsdbClientService.java b/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/OvsdbClientService.java index dd0d7874e7..109f84f922 100644 --- a/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/OvsdbClientService.java +++ b/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/OvsdbClientService.java @@ -55,7 +55,7 @@ public interface OvsdbClientService extends OvsdbRpc { * selectSrcPort or packets containing selectVlan * to mirrorPort or to all ports that trunk mirrorVlan. * - * @param bridgeName the name of the bridge + * @param bridgeName the name of the bridge * @param mirror the OVSDB mirror description * @return true if mirror creation is successful, false otherwise */ @@ -119,14 +119,14 @@ public interface OvsdbClientService extends OvsdbRpc { /** * Gets a qos of node. * @param qosId qos identifier - * @return null if no qos is find + * @return null if no qos is found */ OvsdbQos getQos(QosId qosId); /** * Gets qoses of node. * - * @return set of qoses; empty if no qos is find + * @return set of qoses; empty if no qos is found */ Set getQoses(); @@ -165,14 +165,14 @@ public interface OvsdbClientService extends OvsdbRpc { /** * Gets a queue of node. * @param queueId the queue identifier - * @return null if no queue is find + * @return null if no queue is found */ OvsdbQueue getQueue(QueueId queueId); /** * Gets queues of node. * - * @return set of queues; empty if no queue is find + * @return set of queues; empty if no queue is found */ Set getQueues(); @@ -211,7 +211,7 @@ public interface OvsdbClientService extends OvsdbRpc { /** * Gets bridges of node. * - * @return set of bridges; empty if no bridge is find + * @return set of bridges; empty if no bridge is found */ Set getBridges(); @@ -219,7 +219,7 @@ public interface OvsdbClientService extends OvsdbRpc { * Gets controllers of node. * * @param openflowDeviceId target device id - * @return set of controllers; empty if no controller is find + * @return set of controllers; empty if no controller is found */ Set getControllers(DeviceId openflowDeviceId); @@ -262,14 +262,14 @@ public interface OvsdbClientService extends OvsdbRpc { /** * Gets ports of bridge. * - * @return set of ports; empty if no ports is find + * @return set of ports; empty if no ports is found */ Set getPorts(); /** * Gets interfaces of bridge. * - * @return set of interfaces; empty if no interface is find + * @return set of interfaces; empty if no interface is found */ Set getInterfaces(); @@ -292,7 +292,7 @@ public interface OvsdbClientService extends OvsdbRpc { * Gets the Bridge uuid. * * @param bridgeName bridge name - * @return bridge uuid, empty if no uuid is find + * @return bridge uuid, empty if no uuid is found */ String getBridgeUuid(String bridgeName); @@ -301,7 +301,7 @@ public interface OvsdbClientService extends OvsdbRpc { * * @param portName port name * @param bridgeUuid bridge uuid - * @return port uuid, empty if no uuid is find + * @return port uuid, empty if no uuid is found */ String getPortUuid(String portName, String bridgeUuid); @@ -336,7 +336,7 @@ public interface OvsdbClientService extends OvsdbRpc { * @param dbName database name * @param tableName table name * @param uuid row uuid - * @return row OVSDB row + * @return row, empty if no row is found */ Row getRow(String dbName, String tableName, String uuid); @@ -381,7 +381,7 @@ public interface OvsdbClientService extends OvsdbRpc { * Considered port as created if port's interface table also gets created,irrespective * of ofport value(has errors or not) */ - public List getPorts(List portNames, DeviceId bridgeId); + List getPorts(List portNames, DeviceId bridgeId); /** * Gets error status for the given portNames. @@ -397,20 +397,21 @@ public interface OvsdbClientService extends OvsdbRpc { * * @param dbName db name * @param tblName table name - * @return first table entry + * @return firstRow, first row of the given table from given db if present */ - - public Optional getFirstRow(String dbName, OvsdbTable tblName); + Optional getFirstRow(String dbName, OvsdbTable tblName); /** * Gets device CPU usage in percentage. - * @return device memory usage. + * + * @return cpuStats, empty data as there is no generic way to fetch such stats */ Optional getDeviceCpuUsage(); /** * Gets device memory usage in kilobytes. - * @return device memory usage. + * + * @return memoryStats, empty data as there is no generic way to fetch such stats */ Optional getDeviceMemoryUsage(); diff --git a/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/driver/DefaultOvsdbClient.java b/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/driver/DefaultOvsdbClient.java index c32c34104e..2d8ee68e9a 100644 --- a/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/driver/DefaultOvsdbClient.java +++ b/protocols/ovsdb/api/src/main/java/org/onosproject/ovsdb/controller/driver/DefaultOvsdbClient.java @@ -233,14 +233,6 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ return rowStore; } - /** - * Gets the ovsdb row. - * - * @param dbName the ovsdb database name - * @param tableName the ovsdb table name - * @param uuid the key of the row - * @return row, empty if row is find - */ @Override public Row getRow(String dbName, String tableName, String uuid) { OvsdbTableStore tableStore = getTableStore(dbName); @@ -283,12 +275,6 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ ovsdbStore.createOrUpdateOvsdbStore(dbName, tableStore); } - /** - * Gets the Mirror uuid. - * - * @param mirrorName mirror name - * @return mirror uuid, empty if no uuid is found - */ @Override public String getMirrorUuid(String mirrorName) { DatabaseSchema dbSchema = schema.get(DATABASENAME); @@ -316,12 +302,6 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ return null; } - /** - * Gets mirrors of the device. - * - * @param deviceId target device id - * @return set of mirroring; empty if no mirror is found - */ @Override public Set getMirroringStatistics(DeviceId deviceId) { Uuid bridgeUuid = getBridgeUuid(deviceId); @@ -584,8 +564,20 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ bridgeUuid.value(), c.getRow())); - removeControllers.forEach(c -> deleteConfig(CONTROLLER, UUID, c.getRow().uuid().value(), - BRIDGE, BRIDGE_CONTROLLER, c.getRow().uuid())); + // Controller removal is extremely dangerous operation, because with + // empty controller list, all existing flow rules will be wiped out. + // To harden the setController operation, we need to double check whether + // the updated controller list size is bigger than the remove controller list size + List updatedControllers = getControllers(bridgeUuid); + if (updatedControllers != null && updatedControllers.size() > removeControllers.size()) { + removeControllers.forEach(c -> + deleteConfig(CONTROLLER, UUID, c.getRow().uuid().value(), + BRIDGE, BRIDGE_CONTROLLER, c.getRow().uuid())); + } else { + log.warn("New controllers were not properly configured to OVS " + + "bridge {} or failed to retrieve controller list from OVS " + + "bridge {}", bridgeUuid, bridgeUuid); + } } @Override @@ -951,15 +943,7 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ .collect(Collectors.toSet()); return ovsdbqueues; } - /** - * Creates a mirror port. Mirrors the traffic - * that goes to selectDstPort or comes from - * selectSrcPort or packets containing selectVlan - * to mirrorPort or to all ports that trunk mirrorVlan. - * - * @param mirror the OVSDB mirror description - * @return true if mirror creation is successful, false otherwise - */ + @Override public boolean createMirror(String bridgeName, OvsdbMirror mirror) { @@ -1071,11 +1055,6 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ return true; } - /** - * Drops the configuration for mirror. - * - * @param mirroringName name of mirror to drop - */ @Override public void dropMirror(MirroringName mirroringName) { String mirrorUuid = getMirrorUuid(mirroringName.name()); @@ -1928,13 +1907,6 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ .findFirst().orElse(null); } - /** - * Get first row of given table from given db. - * - * @param dbName db name - * @param tblName table name - * @return firstRow, first row of the given table from given db if present - */ @Override public Optional getFirstRow(String dbName, OvsdbTable tblName) { @@ -1969,22 +1941,12 @@ public class DefaultOvsdbClient implements OvsdbProviderService, OvsdbClientServ } - /** - * Get memory usage of device. - * - * @return memoryStats, empty data as there is no generic way to fetch such stats - */ @Override public Optional getDeviceMemoryUsage() { return Optional.empty(); } - /** - * Get cpu usage of device. - * - * @return cpuStats, empty data as there is no generic way to fetch such stats - */ @Override public Optional getDeviceCpuUsage() { return Optional.empty();