From ea3abd4ff665c8757cae46ea96bdbaae33128edd Mon Sep 17 00:00:00 2001 From: Ray Milkey Date: Tue, 19 Mar 2019 14:22:02 -0700 Subject: [PATCH] cfgdef tool modifications to catch variable names that don't match properties - catch errors in the cfgdef tool and abort compilation if a mismatch is seen - Fix mismatches in the code discovered by the tool Change-Id: Icd9a15eb9312bba6c2208b0b2a684062fcdc19c3 --- .../dhcprelay/Dhcp4HandlerImpl.java | 1 - .../dhcprelay/Dhcp6HandlerImpl.java | 1 - .../store/DistributedFpmPrefixStore.java | 5 +-- .../onosproject/fwd/ReactiveForwarding.java | 10 ++--- .../transactionperf/TransactionPerfApp.java | 2 +- .../impl/DistributedVirtualFlowRuleStore.java | 14 +++---- .../onosproject/core/impl/CoreManager.java | 10 ++--- .../store/OsgiPropertyConstants.java | 2 +- .../impl/OpenFlowControllerImpl.java | 4 +- .../device/impl/GeneralDeviceProvider.java | 40 +++++++++---------- .../lldp/impl/OsgiPropertyConstants.java | 2 +- .../NetworkConfigLinksProvider.java | 4 +- .../netcfglinks/OsgiPropertyConstants.java | 2 +- .../onosproject/cfgdef/CfgDefGenerator.java | 2 +- 14 files changed, 47 insertions(+), 52 deletions(-) diff --git a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java index 38688094c6..990dea853f 100644 --- a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java +++ b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java @@ -123,7 +123,6 @@ import static org.onosproject.net.flowobjective.Objective.Operation.REMOVE; @Component( service = { DhcpHandler.class, HostProvider.class }, property = { - "version:Integer = 4", LEARN_ROUTE_FROM_LEASE_QUERY + ":Boolean=" + LEARN_ROUTE_FROM_LEASE_QUERY_DEFAULT } ) diff --git a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp6HandlerImpl.java b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp6HandlerImpl.java index 7d23293263..5068eaee82 100644 --- a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp6HandlerImpl.java +++ b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp6HandlerImpl.java @@ -128,7 +128,6 @@ import java.util.concurrent.Semaphore; @Component( service = { DhcpHandler.class, HostProvider.class }, property = { - "version:Integer=6", LEARN_ROUTE_FROM_LEASE_QUERY + ":Boolean=" + LEARN_ROUTE_FROM_LEASE_QUERY_DEFAULT } ) diff --git a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/store/DistributedFpmPrefixStore.java b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/store/DistributedFpmPrefixStore.java index bec99e1fab..5a9bbb9eac 100644 --- a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/store/DistributedFpmPrefixStore.java +++ b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/store/DistributedFpmPrefixStore.java @@ -45,10 +45,7 @@ import static org.slf4j.LoggerFactory.getLogger; */ @Component( immediate = true, - service = DhcpFpmPrefixStore.class, - property = { - "fpm_type=DHCP" - } + service = DhcpFpmPrefixStore.class ) public class DistributedFpmPrefixStore implements DhcpFpmPrefixStore { diff --git a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java index 83dc3bc23c..6ed366e419 100644 --- a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java +++ b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java @@ -219,7 +219,7 @@ public class ReactiveForwarding { private boolean matchIcmpFields = MATCH_ICMP_FIELDS_DEFAULT; /** Ignore (do not forward) IPv4 multicast packets; default is false. */ - private boolean ignoreIpv4McastPackets = IGNORE_IPV4_MCAST_PACKETS_DEFAULT; + private boolean ignoreIPv4Multicast = IGNORE_IPV4_MCAST_PACKETS_DEFAULT; /** Enable record metrics for reactive forwarding. */ private boolean recordMetrics = RECORD_METRICS_DEFAULT; @@ -436,11 +436,11 @@ public class ReactiveForwarding { Tools.isPropertyEnabled(properties, IGNORE_IPV4_MCAST_PACKETS); if (ignoreIpv4McastPacketsEnabled == null) { log.info("Ignore IPv4 multi-cast packet is not configured, " + - "using current value of {}", ignoreIpv4McastPackets); + "using current value of {}", ignoreIPv4Multicast); } else { - ignoreIpv4McastPackets = ignoreIpv4McastPacketsEnabled; + ignoreIPv4Multicast = ignoreIpv4McastPacketsEnabled; log.info("Configured. Ignore IPv4 multicast packets is {}", - ignoreIpv4McastPackets ? "enabled" : "disabled"); + ignoreIPv4Multicast ? "enabled" : "disabled"); } Boolean recordMetricsEnabled = Tools.isPropertyEnabled(properties, RECORD_METRICS); @@ -507,7 +507,7 @@ public class ReactiveForwarding { } // Do not process IPv4 multicast packets, let mfwd handle them - if (ignoreIpv4McastPackets && ethPkt.getEtherType() == Ethernet.TYPE_IPV4) { + if (ignoreIPv4Multicast && ethPkt.getEtherType() == Ethernet.TYPE_IPV4) { if (id.mac().isMulticast()) { return; } diff --git a/apps/test/transaction-perf/src/main/java/org/onosproject/transactionperf/TransactionPerfApp.java b/apps/test/transaction-perf/src/main/java/org/onosproject/transactionperf/TransactionPerfApp.java index b79ea2ee90..b41cad1738 100644 --- a/apps/test/transaction-perf/src/main/java/org/onosproject/transactionperf/TransactionPerfApp.java +++ b/apps/test/transaction-perf/src/main/java/org/onosproject/transactionperf/TransactionPerfApp.java @@ -66,7 +66,7 @@ import static org.slf4j.LoggerFactory.getLogger; immediate = true, service = TransactionPerfApp.class, property = { - MAP_NAME_DEFAULT + "=" + MAP_NAME, + MAP_NAME + "=" + MAP_NAME_DEFAULT, READ_PERCENTAGE + ":Double=" + READ_PERCENTAGE_DEFAULT, TOTAL_OPERATIONS + ":Integer=" + TOTAL_OPERATIONS_DEFAULT, WITH_CONTENTION + ":Boolean=" + WITH_CONTENTION_DEFAULT, diff --git a/apps/virtual/app/src/main/java/org/onosproject/incubator/net/virtual/store/impl/DistributedVirtualFlowRuleStore.java b/apps/virtual/app/src/main/java/org/onosproject/incubator/net/virtual/store/impl/DistributedVirtualFlowRuleStore.java index 8722c92720..c43b0b992d 100644 --- a/apps/virtual/app/src/main/java/org/onosproject/incubator/net/virtual/store/impl/DistributedVirtualFlowRuleStore.java +++ b/apps/virtual/app/src/main/java/org/onosproject/incubator/net/virtual/store/impl/DistributedVirtualFlowRuleStore.java @@ -134,7 +134,7 @@ public class DistributedVirtualFlowRuleStore = new MessageSubject("virtual-peer-apply-completed"); /** Number of threads in the message handler pool. */ - private int msgHandlerThreadPoolSize = MESSAGE_HANDLER_THREAD_POOL_SIZE_DEFAULT; + private int messageHandlerThreadPoolSize = MESSAGE_HANDLER_THREAD_POOL_SIZE_DEFAULT; /** Delay in ms between successive backup runs. */ private int backupPeriod = BACKUP_PERIOD_MILLIS_DEFAULT; @@ -197,7 +197,7 @@ public class DistributedVirtualFlowRuleStore eventHandler = Executors.newSingleThreadExecutor( groupedThreads("onos/virtual-flow", "event-handler", log)); messageHandlingExecutor = Executors.newFixedThreadPool( - msgHandlerThreadPoolSize, groupedThreads("onos/store/virtual-flow", "message-handlers", log)); + messageHandlerThreadPoolSize, groupedThreads("onos/store/virtual-flow", "message-handlers", log)); registerMessageHandlers(messageHandlingExecutor); @@ -238,7 +238,7 @@ public class DistributedVirtualFlowRuleStore int newBackupPeriod; try { String s = get(properties, MESSAGE_HANDLER_THREAD_POOL_SIZE); - newPoolSize = isNullOrEmpty(s) ? msgHandlerThreadPoolSize : Integer.parseInt(s.trim()); + newPoolSize = isNullOrEmpty(s) ? messageHandlerThreadPoolSize : Integer.parseInt(s.trim()); s = get(properties, BACKUP_PERIOD_MILLIS); newBackupPeriod = isNullOrEmpty(s) ? backupPeriod : Integer.parseInt(s.trim()); @@ -256,11 +256,11 @@ public class DistributedVirtualFlowRuleStore if (restartBackupTask) { log.warn("Currently, backup tasks are not supported."); } - if (newPoolSize != msgHandlerThreadPoolSize) { - msgHandlerThreadPoolSize = newPoolSize; + if (newPoolSize != messageHandlerThreadPoolSize) { + messageHandlerThreadPoolSize = newPoolSize; ExecutorService oldMsgHandler = messageHandlingExecutor; messageHandlingExecutor = Executors.newFixedThreadPool( - msgHandlerThreadPoolSize, groupedThreads("onos/store/virtual-flow", "message-handlers", log)); + messageHandlerThreadPoolSize, groupedThreads("onos/store/virtual-flow", "message-handlers", log)); // replace previously registered handlers. registerMessageHandlers(messageHandlingExecutor); @@ -566,7 +566,7 @@ public class DistributedVirtualFlowRuleStore private void logConfig(String prefix) { log.info("{} with msgHandlerPoolSize = {}; backupPeriod = {}", - prefix, msgHandlerThreadPoolSize, backupPeriod); + prefix, messageHandlerThreadPoolSize, backupPeriod); } private void storeBatchInternal(NetworkId networkId, FlowRuleBatchOperation operation) { diff --git a/core/net/src/main/java/org/onosproject/core/impl/CoreManager.java b/core/net/src/main/java/org/onosproject/core/impl/CoreManager.java index e82efd7983..5d3ed3da5c 100644 --- a/core/net/src/main/java/org/onosproject/core/impl/CoreManager.java +++ b/core/net/src/main/java/org/onosproject/core/impl/CoreManager.java @@ -97,7 +97,7 @@ public class CoreManager implements CoreService { private int maxEventTimeLimit = MAX_EVENT_TIME_LIMIT_DEFAULT; /** Enable queue performance check on shared pool. */ - private boolean calculatePoolPerformance = CALCULATE_PERFORMANCE_CHECK_DEFAULT; + private boolean sharedThreadPerformanceCheck = CALCULATE_PERFORMANCE_CHECK_DEFAULT; @Activate @@ -184,11 +184,11 @@ public class CoreManager implements CoreService { Boolean performanceCheck = Tools.isPropertyEnabled(properties, CALCULATE_PERFORMANCE_CHECK); if (performanceCheck != null) { - calculatePoolPerformance = performanceCheck; - SharedExecutors.setMetricsService(calculatePoolPerformance ? metricsService : null); + sharedThreadPerformanceCheck = performanceCheck; + SharedExecutors.setMetricsService(sharedThreadPerformanceCheck ? metricsService : null); } - log.info("Settings: sharedThreadPoolSize={}, maxEventTimeLimit={}, calculatePoolPerformance={}", - sharedThreadPoolSize, maxEventTimeLimit, calculatePoolPerformance); + log.info("Settings: sharedThreadPoolSize={}, maxEventTimeLimit={}, sharedThreadPerformanceCheck={}", + sharedThreadPoolSize, maxEventTimeLimit, sharedThreadPerformanceCheck); } } diff --git a/core/store/dist/src/main/java/org/onosproject/store/OsgiPropertyConstants.java b/core/store/dist/src/main/java/org/onosproject/store/OsgiPropertyConstants.java index 1f4555db12..9266bf695e 100644 --- a/core/store/dist/src/main/java/org/onosproject/store/OsgiPropertyConstants.java +++ b/core/store/dist/src/main/java/org/onosproject/store/OsgiPropertyConstants.java @@ -32,7 +32,7 @@ public final class OsgiPropertyConstants { public static final String ANTI_ENTROPY_PERIOD_MILLIS = "antiEntropyPeriod"; public static final int ANTI_ENTROPY_PERIOD_MILLIS_DEFAULT = 5000; - public static final String EC_FLOW_RULE_STORE_PERSISTENCE_ENABLED = "flowRuleStorePersistenceEnabled"; + public static final String EC_FLOW_RULE_STORE_PERSISTENCE_ENABLED = "persistenceEnabled"; public static final boolean EC_FLOW_RULE_STORE_PERSISTENCE_ENABLED_DEFAULT = false; public static final String MAX_BACKUP_COUNT = "backupCount"; diff --git a/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java b/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java index 2d5213d07d..ee9422f85a 100644 --- a/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java +++ b/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java @@ -126,13 +126,13 @@ public class OpenFlowControllerImpl implements OpenFlowController { protected NetworkConfigRegistry netCfgService; /** Port numbers (comma separated) used by OpenFlow protocol; default is 6633,6653. */ - private String openflowPortsValue = OFPORTS_DEFAULT; + private String openflowPorts = OFPORTS_DEFAULT; /** Number of controller worker threads. */ private int workerThreads = WORKER_THREADS_DEFAULT; /** TLS mode for OpenFlow channel; options are: disabled [default], enabled, strict. */ - private String tlsModeString; + private String tlsMode; /** File path to key store for TLS connections. */ private String keyStore; diff --git a/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java b/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java index 1eaa808755..d977c99d00 100644 --- a/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java +++ b/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java @@ -183,18 +183,18 @@ public class GeneralDeviceProvider extends AbstractProvider /** * Configure poll frequency for port status and statistics; default is 10 sec. */ - private int statsPollFrequency = STATS_POLL_FREQUENCY_DEFAULT; + private int deviceStatsPollFrequency = STATS_POLL_FREQUENCY_DEFAULT; /** * Configure probe frequency for checking device availability; default is 10 sec. */ - private int probeFrequency = PROBE_FREQUENCY_DEFAULT; + private int deviceProbeFrequency = PROBE_FREQUENCY_DEFAULT; /** * Configure timeout in seconds for device operations that are supposed to take a short time * (e.g. checking device reachability); default is 10 seconds. */ - private int opTimeoutShort = OP_TIMEOUT_SHORT_DEFAULT; + private int deviceOperationTimeoutShort = OP_TIMEOUT_SHORT_DEFAULT; //FIXME to be removed when netcfg will issue device events in a bundle or //ensures all configuration needed is present @@ -252,26 +252,26 @@ public class GeneralDeviceProvider extends AbstractProvider } Dictionary properties = context.getProperties(); - final int oldStatsPollFrequency = statsPollFrequency; - statsPollFrequency = Tools.getIntegerProperty( + final int oldStatsPollFrequency = deviceStatsPollFrequency; + deviceStatsPollFrequency = Tools.getIntegerProperty( properties, STATS_POLL_FREQUENCY, STATS_POLL_FREQUENCY_DEFAULT); log.info("Configured. {} is configured to {} seconds", - STATS_POLL_FREQUENCY, statsPollFrequency); - final int oldProbeFrequency = probeFrequency; - probeFrequency = Tools.getIntegerProperty( + STATS_POLL_FREQUENCY, deviceStatsPollFrequency); + final int oldProbeFrequency = deviceProbeFrequency; + deviceProbeFrequency = Tools.getIntegerProperty( properties, PROBE_FREQUENCY, PROBE_FREQUENCY_DEFAULT); log.info("Configured. {} is configured to {} seconds", - PROBE_FREQUENCY, probeFrequency); - opTimeoutShort = Tools.getIntegerProperty( + PROBE_FREQUENCY, deviceProbeFrequency); + deviceOperationTimeoutShort = Tools.getIntegerProperty( properties, OP_TIMEOUT_SHORT, OP_TIMEOUT_SHORT_DEFAULT); log.info("Configured. {} is configured to {} seconds", - OP_TIMEOUT_SHORT, opTimeoutShort); + OP_TIMEOUT_SHORT, deviceOperationTimeoutShort); - if (oldStatsPollFrequency != statsPollFrequency) { + if (oldStatsPollFrequency != deviceStatsPollFrequency) { rescheduleStatsPollingTasks(); } - if (oldProbeFrequency != probeFrequency) { + if (oldProbeFrequency != deviceProbeFrequency) { rescheduleProbeTask(true); } } @@ -283,8 +283,8 @@ public class GeneralDeviceProvider extends AbstractProvider } probeTask = probeExecutor.scheduleAtFixedRate( this::triggerProbeAllDevices, - deelay ? probeFrequency : 0, - probeFrequency, + deelay ? deviceProbeFrequency : 0, + deviceProbeFrequency, TimeUnit.SECONDS); } } @@ -370,7 +370,7 @@ public class GeneralDeviceProvider extends AbstractProvider } return getFutureWithDeadline( handshaker.isReachable(), "checking reachability", - deviceId, false, opTimeoutShort); + deviceId, false, deviceOperationTimeoutShort); } private boolean isConnected(DeviceId deviceId) { @@ -404,7 +404,7 @@ public class GeneralDeviceProvider extends AbstractProvider : portAdmin.disable(portNumber); final String descr = (enable ? "enabling" : "disabling") + " port " + portNumber; getFutureWithDeadline( - modifyTask, descr, deviceId, null, opTimeoutShort); + modifyTask, descr, deviceId, null, deviceOperationTimeoutShort); } @Override @@ -475,7 +475,7 @@ public class GeneralDeviceProvider extends AbstractProvider // Start connection via handshaker. final Boolean connectSuccess = getFutureWithDeadline( handshaker.connect(), "initiating connection", - deviceId, false, opTimeoutShort); + deviceId, false, deviceOperationTimeoutShort); if (!connectSuccess) { log.warn("Unable to connect to {}", deviceId); } @@ -652,7 +652,7 @@ public class GeneralDeviceProvider extends AbstractProvider handshakersWithListeners.remove(deviceId); final boolean disconnectSuccess = getFutureWithDeadline( handshaker.disconnect(), "performing disconnection", - deviceId, false, opTimeoutShort); + deviceId, false, deviceOperationTimeoutShort); if (!disconnectSuccess) { log.warn("Unable to disconnect from {}", deviceId); } @@ -831,7 +831,7 @@ public class GeneralDeviceProvider extends AbstractProvider ? new SecureRandom().nextInt(10) : 0; return statsExecutor.scheduleAtFixedRate( exceptionSafe(() -> updatePortStatistics(deviceId)), - delay, statsPollFrequency, TimeUnit.SECONDS); + delay, deviceStatsPollFrequency, TimeUnit.SECONDS); }); } diff --git a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/OsgiPropertyConstants.java b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/OsgiPropertyConstants.java index 77e716794a..51bcd396d3 100644 --- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/OsgiPropertyConstants.java +++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/OsgiPropertyConstants.java @@ -26,7 +26,7 @@ public final class OsgiPropertyConstants { public static final String PROP_ENABLED = "enabled"; public static final boolean ENABLED_DEFAULT = true; - public static final String PROP_USE_BDDP = "useBDDP"; + public static final String PROP_USE_BDDP = "useBddp"; public static final boolean USE_BDDP_DEFAULT = true; public static final String PROP_PROBE_RATE = "probeRate"; diff --git a/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java b/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java index 68e489574b..e73163cf9f 100644 --- a/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java +++ b/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/NetworkConfigLinksProvider.java @@ -112,7 +112,7 @@ public class NetworkConfigLinksProvider private int probeRate = PROBE_RATE_DEFAULT; /** Number of millis beyond which an LLDP packet will not be accepted. */ - private int maxDiscoveryDelayMs = DISCOVERY_DELAY_DEFAULT; + private int maxLldpAge = DISCOVERY_DELAY_DEFAULT; // Device link discovery helpers. protected final Map discoverers = new ConcurrentHashMap<>(); @@ -284,7 +284,7 @@ public class NetworkConfigLinksProvider @Override public long maxDiscoveryDelay() { - return maxDiscoveryDelayMs; + return maxLldpAge; } } diff --git a/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/OsgiPropertyConstants.java b/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/OsgiPropertyConstants.java index 5d718eda90..463ae254ae 100644 --- a/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/OsgiPropertyConstants.java +++ b/providers/netcfglinks/src/main/java/org/onosproject/provider/netcfglinks/OsgiPropertyConstants.java @@ -26,7 +26,7 @@ public final class OsgiPropertyConstants { public static final String PROP_PROBE_RATE = "probeRate"; public static final int PROBE_RATE_DEFAULT = 3000; - public static final String PROP_DISCOVERY_DELAY = "maxLLDPAge"; + public static final String PROP_DISCOVERY_DELAY = "maxLldpAge"; public static final int DISCOVERY_DELAY_DEFAULT = 1000; } diff --git a/tools/build/cfgdef/src/main/java/org/onosproject/cfgdef/CfgDefGenerator.java b/tools/build/cfgdef/src/main/java/org/onosproject/cfgdef/CfgDefGenerator.java index c46b583c00..6b7017d281 100644 --- a/tools/build/cfgdef/src/main/java/org/onosproject/cfgdef/CfgDefGenerator.java +++ b/tools/build/cfgdef/src/main/java/org/onosproject/cfgdef/CfgDefGenerator.java @@ -147,7 +147,7 @@ public class CfgDefGenerator { String comment = field.getComment(); return comment != null ? comment : NO_DESCRIPTION; } - return null; + throw new IllegalStateException("cfgdef could not find a variable named " + name + " in " + javaClass.getName()); } private String elaborate(AnnotationValue value) {