Fixes for stricter sca rules
Change-Id: I027796c040009ec21d9864b1868757993d47cb35
diff --git a/.golangci.yml b/.golangci.yml
index 5f2cae9..058bf20 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -15,12 +15,24 @@
run:
modules-download-mode: vendor
+issues:
+ exclude-use-default: false #we should decide ourselves about false positives
+ exclude-rules:
+ - path: _test\.go
+ linters:
+ - errcheck
+ - gocritic
+ - gosec
+ - linters:
+ - gocritic
+ text: "ifElseChain:" #it should be up to a developer to decide which operator to use
+
linters:
enable:
- #- gocritic
- - gofmt
- - golint
- - gosec
- #- unparam
#- gochecknoglobals
#- gochecknoinits
+ - gocritic
+ - gofmt
+ - gosec
+ - errcheck
+ #- unparam
diff --git a/VERSION b/VERSION
index e0c9c37..5d9ade1 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.9.2-dev
+2.9.2
diff --git a/db/model/proxy_test.go b/db/model/proxy_test.go
index f504ed7..4f97d49 100644
--- a/db/model/proxy_test.go
+++ b/db/model/proxy_test.go
@@ -171,7 +171,9 @@
}
d := &voltha.Device{}
- if have, err := TestProxyRootDevice.Get(context.Background(), TestProxyDeviceID, d); err != nil {
+
+ have, err := TestProxyRootDevice.Get(context.Background(), TestProxyDeviceID, d)
+ if err != nil {
BenchmarkProxyLogger.Errorf(ctx, "Failed get device info from test proxy due to error: %v", err)
assert.NotNil(t, err)
} else if !have {
@@ -385,15 +387,18 @@
}
device := &voltha.LogicalDevice{}
- if have, err := TestProxyRootLogicalDevice.Get(context.Background(), "logical_devices", device); err != nil {
+ have, err := TestProxyRootLogicalDevice.Get(context.Background(), "logical_devices", device)
+ if err != nil {
BenchmarkProxyLogger.Errorf(ctx, "Failed to get logical device info from logical device proxy due to error: %v", err)
assert.NotNil(t, err)
- } else if !have {
+ return
+ }
+ if !have {
t.Error("Failed to find added logical device")
- } else {
- if device.String() != TestProxyLogicalDevice.String() {
- t.Errorf("Logical devices don't match - existing: %+v returned: %+v", TestProxyLogicalDevice, device)
- }
+ return
+ }
+ if device.String() != TestProxyLogicalDevice.String() {
+ t.Errorf("Logical devices don't match - existing: %+v returned: %+v", TestProxyLogicalDevice, device)
}
}
diff --git a/rw_core/config/config.go b/rw_core/config/config.go
index 6510964..5c092d6 100644
--- a/rw_core/config/config.go
+++ b/rw_core/config/config.go
@@ -18,7 +18,6 @@
import (
"flag"
- "fmt"
"time"
)
@@ -138,94 +137,65 @@
// ParseCommandArguments parses the arguments when running read-write core service
func (cf *RWCoreFlags) ParseCommandArguments() {
- help := fmt.Sprintf("RW core endpoint address")
- flag.StringVar(&(cf.RWCoreEndpoint), "vcore-endpoint", defaultRWCoreEndpoint, help)
+ flag.StringVar(&(cf.RWCoreEndpoint), "vcore-endpoint", defaultRWCoreEndpoint, "RW core endpoint address")
- help = fmt.Sprintf("GRPC server - address")
- flag.StringVar(&(cf.GrpcAddress), "grpc_address", defaultGrpcAddress, help)
+ flag.StringVar(&(cf.GrpcAddress), "grpc_address", defaultGrpcAddress, "GRPC server - address")
- help = fmt.Sprintf("Kafka - Adapter messaging address")
- flag.StringVar(&(cf.KafkaAdapterAddress), "kafka_adapter_address", defaultKafkaAdapterAddress, help)
+ flag.StringVar(&(cf.KafkaAdapterAddress), "kafka_adapter_address", defaultKafkaAdapterAddress, "Kafka - Adapter messaging address")
- help = fmt.Sprintf("Kafka - Cluster messaging address")
- flag.StringVar(&(cf.KafkaClusterAddress), "kafka_cluster_address", defaultKafkaClusterAddress, help)
+ flag.StringVar(&(cf.KafkaClusterAddress), "kafka_cluster_address", defaultKafkaClusterAddress, "Kafka - Cluster messaging address")
- help = fmt.Sprintf("RW Core topic")
- flag.StringVar(&(cf.CoreTopic), "rw_core_topic", defaultCoreTopic, help)
+ flag.StringVar(&(cf.CoreTopic), "rw_core_topic", defaultCoreTopic, "RW Core topic")
- help = fmt.Sprintf("RW Core Event topic")
- flag.StringVar(&(cf.EventTopic), "event_topic", defaultEventTopic, help)
+ flag.StringVar(&(cf.EventTopic), "event_topic", defaultEventTopic, "RW Core Event topic")
flag.Bool("in_competing_mode", false, "deprecated")
- help = fmt.Sprintf("KV store type")
- flag.StringVar(&(cf.KVStoreType), "kv_store_type", defaultKVStoreType, help)
+ flag.StringVar(&(cf.KVStoreType), "kv_store_type", defaultKVStoreType, "KV store type")
- help = fmt.Sprintf("The default timeout when making a kv store request")
- flag.DurationVar(&(cf.KVStoreTimeout), "kv_store_request_timeout", defaultKVStoreTimeout, help)
+ flag.DurationVar(&(cf.KVStoreTimeout), "kv_store_request_timeout", defaultKVStoreTimeout, "The default timeout when making a kv store request")
- help = fmt.Sprintf("KV store address")
- flag.StringVar(&(cf.KVStoreAddress), "kv_store_address", defaultKVStoreAddress, help)
+ flag.StringVar(&(cf.KVStoreAddress), "kv_store_address", defaultKVStoreAddress, "KV store address")
- help = fmt.Sprintf("The time to wait before deleting a completed transaction key")
- flag.IntVar(&(cf.KVTxnKeyDelTime), "kv_txn_delete_time", defaultKVTxnKeyDelTime, help)
+ flag.IntVar(&(cf.KVTxnKeyDelTime), "kv_txn_delete_time", defaultKVTxnKeyDelTime, "The time to wait before deleting a completed transaction key")
- help = fmt.Sprintf("Log level")
- flag.StringVar(&(cf.LogLevel), "log_level", defaultLogLevel, help)
+ flag.StringVar(&(cf.LogLevel), "log_level", defaultLogLevel, "Log level")
- help = fmt.Sprintf("Timeout for long running request")
- flag.DurationVar(&(cf.LongRunningRequestTimeout), "timeout_long_request", defaultLongRunningRequestTimeout, help)
+ flag.DurationVar(&(cf.LongRunningRequestTimeout), "timeout_long_request", defaultLongRunningRequestTimeout, "Timeout for long running request")
- help = fmt.Sprintf("Default timeout for regular request")
- flag.DurationVar(&(cf.DefaultRequestTimeout), "timeout_request", defaultDefaultRequestTimeout, help)
+ flag.DurationVar(&(cf.DefaultRequestTimeout), "timeout_request", defaultDefaultRequestTimeout, "Default timeout for regular request")
- help = fmt.Sprintf("Default Core timeout")
- flag.DurationVar(&(cf.DefaultCoreTimeout), "core_timeout", defaultCoreTimeout, help)
+ flag.DurationVar(&(cf.DefaultCoreTimeout), "core_timeout", defaultCoreTimeout, "Default Core timeout")
- help = fmt.Sprintf("Show startup banner log lines")
- flag.BoolVar(&cf.Banner, "banner", defaultBanner, help)
+ flag.BoolVar(&cf.Banner, "banner", defaultBanner, "Show startup banner log lines")
- help = fmt.Sprintf("Show version information and exit")
- flag.BoolVar(&cf.DisplayVersionOnly, "version", defaultDisplayVersionOnly, help)
+ flag.BoolVar(&cf.DisplayVersionOnly, "version", defaultDisplayVersionOnly, "Show version information and exit")
- help = fmt.Sprintf("The name of the meta-key whose value is the rw-core group to which the ofagent is bound")
- flag.StringVar(&(cf.CoreBindingKey), "core_binding_key", defaultCoreBindingKey, help)
+ flag.StringVar(&(cf.CoreBindingKey), "core_binding_key", defaultCoreBindingKey, "The name of the meta-key whose value is the rw-core group to which the ofagent is bound")
- help = fmt.Sprintf("The number of retries to connect to a dependent component")
- flag.IntVar(&(cf.MaxConnectionRetries), "max_connection_retries", defaultMaxConnectionRetries, help)
+ flag.IntVar(&(cf.MaxConnectionRetries), "max_connection_retries", defaultMaxConnectionRetries, "The number of retries to connect to a dependent component")
- help = fmt.Sprintf("The number of seconds between each connection retry attempt")
- flag.DurationVar(&(cf.ConnectionRetryInterval), "connection_retry_interval", defaultConnectionRetryInterval, help)
+ flag.DurationVar(&(cf.ConnectionRetryInterval), "connection_retry_interval", defaultConnectionRetryInterval, "The number of seconds between each connection retry attempt")
- help = fmt.Sprintf("The number of seconds between liveness probes while in a live state")
- flag.DurationVar(&(cf.LiveProbeInterval), "live_probe_interval", defaultLiveProbeInterval, help)
+ flag.DurationVar(&(cf.LiveProbeInterval), "live_probe_interval", defaultLiveProbeInterval, "The number of seconds between liveness probes while in a live state")
- help = fmt.Sprintf("The number of seconds between liveness probes while in a not live state")
- flag.DurationVar(&(cf.NotLiveProbeInterval), "not_live_probe_interval", defaultNotLiveProbeInterval, help)
+ flag.DurationVar(&(cf.NotLiveProbeInterval), "not_live_probe_interval", defaultNotLiveProbeInterval, "The number of seconds between liveness probes while in a not live state")
- help = fmt.Sprintf("The address on which to listen to answer liveness and readiness probe queries over HTTP.")
- flag.StringVar(&(cf.ProbeAddress), "probe_address", defaultProbeAddress, help)
+ flag.StringVar(&(cf.ProbeAddress), "probe_address", defaultProbeAddress, "The address on which to listen to answer liveness and readiness probe queries over HTTP")
- help = fmt.Sprintf("Whether to send logs to tracing agent?")
- flag.BoolVar(&(cf.TraceEnabled), "trace_enabled", defaultTraceEnabled, help)
+ flag.BoolVar(&(cf.TraceEnabled), "trace_enabled", defaultTraceEnabled, "Whether to send logs to tracing agent?")
- help = fmt.Sprintf("The address of tracing agent to which span info should be sent.")
- flag.StringVar(&(cf.TraceAgentAddress), "trace_agent_address", defaultTraceAgentAddress, help)
+ flag.StringVar(&(cf.TraceAgentAddress), "trace_agent_address", defaultTraceAgentAddress, "The address of tracing agent to which span info should be sent")
- help = fmt.Sprintf("Whether to enrich log statements with fields denoting operation being executed for achieving correlation?")
- flag.BoolVar(&(cf.LogCorrelationEnabled), "log_correlation_enabled", defaultLogCorrelationEnabled, help)
+ flag.BoolVar(&(cf.LogCorrelationEnabled), "log_correlation_enabled", defaultLogCorrelationEnabled, "Whether to enrich log statements with fields denoting operation being executed for achieving correlation?")
- help = fmt.Sprintf("ID for the current voltha stack")
- flag.StringVar(&cf.VolthaStackID, "stack_id", defaultVolthaStackID, help)
+ flag.StringVar(&cf.VolthaStackID, "stack_id", defaultVolthaStackID, "ID for the current voltha stack")
- help = fmt.Sprintf("The initial number of milliseconds an exponential backoff will wait before a retry")
- flag.DurationVar(&(cf.BackoffRetryInitialInterval), "backoff_retry_initial_interval", defaultBackoffRetryInitialInterval, help)
+ flag.DurationVar(&(cf.BackoffRetryInitialInterval), "backoff_retry_initial_interval", defaultBackoffRetryInitialInterval, "The initial number of milliseconds an exponential backoff will wait before a retry")
- help = fmt.Sprintf("The maximum number of milliseconds an exponential backoff can elasped")
- flag.DurationVar(&(cf.BackoffRetryMaxElapsedTime), "backoff_retry_max_elapsed_time", defaultBackoffRetryMaxElapsedTime, help)
+ flag.DurationVar(&(cf.BackoffRetryMaxElapsedTime), "backoff_retry_max_elapsed_time", defaultBackoffRetryMaxElapsedTime, "The maximum number of milliseconds an exponential backoff can elasped")
- help = fmt.Sprintf("The maximum number of milliseconds of an exponential backoff interval")
- flag.DurationVar(&(cf.BackoffRetryMaxInterval), "backoff_retry_max_interval", defaultBackoffRetryMaxInterval, help)
+ flag.DurationVar(&(cf.BackoffRetryMaxInterval), "backoff_retry_max_interval", defaultBackoffRetryMaxInterval, "The maximum number of milliseconds of an exponential backoff interval")
flag.Parse()
}
diff --git a/rw_core/core/api/grpc_nbi_handler_test.go b/rw_core/core/api/grpc_nbi_handler_test.go
index 8d9cb16..9ad3926 100755
--- a/rw_core/core/api/grpc_nbi_handler_test.go
+++ b/rw_core/core/api/grpc_nbi_handler_test.go
@@ -1140,8 +1140,12 @@
}
// Wait for logical device to have the flows (or none
var vlFunction isLogicalDevicesConditionSatisfied = func(lds *voltha.LogicalDevices) bool {
- flows, _ := nbi.ListLogicalDeviceFlows(getContext(), &voltha.ID{Id: lds.Items[0].Id})
- return lds != nil && len(lds.Items) == 1 && len(flows.Items) == expectedNumFlows
+ id := ""
+ if lds != nil {
+ id = lds.Items[0].Id
+ }
+ flws, _ := nbi.ListLogicalDeviceFlows(getContext(), &voltha.ID{Id: id})
+ return lds != nil && len(lds.Items) == 1 && len(flws.Items) == expectedNumFlows
}
// No timeout implies a success
err := waitUntilConditionForLogicalDevices(nb.maxTimeout, nbi, vlFunction)
@@ -1430,7 +1434,12 @@
if err != nil {
logger.Fatalf(ctx, "could not create CPU profile: %v\n ", err)
}
- defer f.Close()
+ defer func() {
+ err = f.Close()
+ if err != nil {
+ logger.Errorf(ctx, "failed to close file: %v\n", err)
+ }
+ }()
runtime.SetBlockProfileRate(1)
runtime.SetMutexProfileFraction(-1)
if err := pprof.StartCPUProfile(f); err != nil {
diff --git a/rw_core/core/device/agent_flow.go b/rw_core/core/device/agent_flow.go
index 4589b91..3b04c02 100644
--- a/rw_core/core/device/agent_flow.go
+++ b/rw_core/core/device/agent_flow.go
@@ -231,7 +231,7 @@
return coreutils.DoneResponse(), status.Errorf(codes.Aborted, "%s", err)
}
if device.OperStatus != voltha.OperStatus_ACTIVE || device.ConnectStatus != voltha.ConnectStatus_REACHABLE || device.AdminState != voltha.AdminState_ENABLED {
- desc = fmt.Sprint("invalid device states")
+ desc = "invalid device states"
agent.logDeviceUpdate(ctx, "updateFlowsToAdapter", nil, nil, operStatus, &desc)
return coreutils.DoneResponse(), status.Errorf(codes.FailedPrecondition, "invalid device states")
}
diff --git a/rw_core/core/device/agent_image.go b/rw_core/core/device/agent_image.go
index 74a60c7..075e2bf 100644
--- a/rw_core/core/device/agent_image.go
+++ b/rw_core/core/device/agent_image.go
@@ -64,7 +64,7 @@
cloned := agent.cloneDeviceWithoutLock()
_, index, err := getImage(img, device)
if err != nil {
- cloned.ImageDownloads = append(device.ImageDownloads, clonedImg)
+ cloned.ImageDownloads = append(cloned.ImageDownloads, clonedImg)
} else {
cloned.ImageDownloads[index] = clonedImg
}
diff --git a/rw_core/core/device/event/event.go b/rw_core/core/device/event/event.go
index 00e08d2..2eeb70a 100644
--- a/rw_core/core/device/event/event.go
+++ b/rw_core/core/device/event/event.go
@@ -100,13 +100,13 @@
func (q *Manager) flushFailedPackets(ctx context.Context, tracker *callTracker) error {
if tracker.failedPacket != nil {
- switch tracker.failedPacket.(type) {
+ switch failedPacket := tracker.failedPacket.(type) {
case openflow_13.PacketIn:
logger.Debug(ctx, "enqueueing-last-failed-packet-in")
- q.packetInQueue <- tracker.failedPacket.(openflow_13.PacketIn)
+ q.packetInQueue <- failedPacket
case openflow_13.ChangeEvent:
logger.Debug(ctx, "enqueueing-last-failed-change-event")
- q.changeEventQueue <- tracker.failedPacket.(openflow_13.ChangeEvent)
+ q.changeEventQueue <- failedPacket
}
}
return nil
@@ -138,11 +138,9 @@
nil, time.Now().Unix())
// save the last failed packet in
streamingTracker.failedPacket = packet
- } else {
- if streamingTracker.failedPacket != nil {
- // reset last failed packet saved to avoid flush
- streamingTracker.failedPacket = nil
- }
+ } else if streamingTracker.failedPacket != nil {
+ // reset last failed packet saved to avoid flush
+ streamingTracker.failedPacket = nil
}
case <-q.packetInQueueDone:
logger.Debug(ctx, "another-receive-packets-in-running-bailing-out")
@@ -228,11 +226,9 @@
time.Now().Unix())
// save last failed change event
streamingTracker.failedPacket = event
- } else {
- if streamingTracker.failedPacket != nil {
- // reset last failed event saved on success to avoid flushing
- streamingTracker.failedPacket = nil
- }
+ } else if streamingTracker.failedPacket != nil {
+ // reset last failed event saved on success to avoid flushing
+ streamingTracker.failedPacket = nil
}
case <-q.changeEventQueueDone:
logger.Debug(ctx, "another-receive-change-events-already-running-bailing-out")
diff --git a/rw_core/core/device/flow/loader_test.go b/rw_core/core/device/flow/loader_test.go
index 958124d..536a447 100644
--- a/rw_core/core/device/flow/loader_test.go
+++ b/rw_core/core/device/flow/loader_test.go
@@ -18,8 +18,10 @@
import (
"bufio"
+ "context"
"fmt"
"os"
+ "path/filepath"
"regexp"
"strconv"
"testing"
@@ -59,17 +61,27 @@
}
func compare(regexesA, regexesB []*regexp.Regexp, fileNameA, fileNameB string) error {
- fileA, err := os.Open(fileNameA)
+ fileA, err := os.Open(filepath.Clean(fileNameA))
if err != nil {
return err
}
- defer fileA.Close()
+ defer func() {
+ err := fileA.Close()
+ if err != nil {
+ logger.Errorf(context.Background(), "failed to close file: %v", err)
+ }
+ }()
- fileB, err := os.Open(fileNameB)
+ fileB, err := os.Open(filepath.Clean(fileNameB))
if err != nil {
return err
}
- defer fileB.Close()
+ defer func() {
+ err := fileB.Close()
+ if err != nil {
+ logger.Errorf(context.Background(), "failed to close file: %v", err)
+ }
+ }()
scannerA, scannerB := bufio.NewScanner(fileA), bufio.NewScanner(fileB)
diff --git a/rw_core/core/device/logical_agent_meter.go b/rw_core/core/device/logical_agent_meter.go
index aad005f..f3a1c89 100644
--- a/rw_core/core/device/logical_agent_meter.go
+++ b/rw_core/core/device/logical_agent_meter.go
@@ -59,10 +59,11 @@
}
func (agent *LogicalAgent) meterAdd(ctx context.Context, meterMod *ofp.OfpMeterMod) error {
- logger.Debugw(ctx, "meterAdd", log.Fields{"metermod": *meterMod})
if meterMod == nil {
+ logger.Errorw(ctx, "failed-meterAdd-meterMod-is-nil", log.Fields{"logical-device-id": agent.logicalDeviceID})
return nil
}
+ logger.Debugw(ctx, "meterAdd", log.Fields{"metermod": *meterMod, "logical-device-id": agent.logicalDeviceID})
meterEntry := fu.MeterEntryFromMeterMod(ctx, meterMod)
@@ -73,22 +74,23 @@
defer meterHandle.Unlock()
if created {
- logger.Debugw(ctx, "Meter-added-successfully", log.Fields{"Added-meter": meterEntry})
+ logger.Debugw(ctx, "Meter-added-successfully", log.Fields{"Added-meter": meterEntry, "logical-device-id": agent.logicalDeviceID})
} else {
- logger.Infow(ctx, "Meter-already-exists", log.Fields{"meter": *meterMod})
+ logger.Infow(ctx, "Meter-already-exists", log.Fields{"meter": *meterMod, "logical-device-id": agent.logicalDeviceID})
}
return nil
}
func (agent *LogicalAgent) meterDelete(ctx context.Context, meterMod *ofp.OfpMeterMod) error {
- logger.Debug(ctx, "meterDelete", log.Fields{"meterMod": *meterMod})
if meterMod == nil {
+ logger.Errorw(ctx, "failed-meterDelete-meterMod-is-nil", log.Fields{"logical-device-id": agent.logicalDeviceID})
return nil
}
+ logger.Debug(ctx, "meterDelete", log.Fields{"meterMod": *meterMod, "logical-device-id": agent.logicalDeviceID})
meterHandle, have := agent.meterLoader.Lock(meterMod.MeterId)
if !have {
- logger.Warnw(ctx, "meter-not-found", log.Fields{"meterID": meterMod.MeterId})
+ logger.Warnw(ctx, "meter-not-found", log.Fields{"meterID": meterMod.MeterId, "logical-device-id": agent.logicalDeviceID})
return nil
}
defer meterHandle.Unlock()
@@ -103,19 +105,20 @@
return err
}
- logger.Debugw(ctx, "meterDelete-success", log.Fields{"meterID": meterMod.MeterId})
+ logger.Debugw(ctx, "meterDelete-success", log.Fields{"meterID": meterMod.MeterId, "logical-device-id": agent.logicalDeviceID})
return nil
}
func (agent *LogicalAgent) meterModify(ctx context.Context, meterMod *ofp.OfpMeterMod) error {
logger.Debug(ctx, "meterModify")
if meterMod == nil {
+ logger.Errorw(ctx, "failed-meterModify-meterMod-is-nil", log.Fields{"logical-device-id": agent.logicalDeviceID})
return nil
}
meterHandle, have := agent.meterLoader.Lock(meterMod.MeterId)
if !have {
- return fmt.Errorf("no-meter-to-modify: %d", meterMod.MeterId)
+ return fmt.Errorf("no-meter-to-modify: %d, logical-device-id: %s", meterMod.MeterId, agent.logicalDeviceID)
}
defer meterHandle.Unlock()
@@ -126,6 +129,6 @@
if err := meterHandle.Update(ctx, newMeter); err != nil {
return err
}
- logger.Debugw(ctx, "replaced-with-new-meter", log.Fields{"oldMeter": oldMeter, "newMeter": newMeter})
+ logger.Debugw(ctx, "replaced-with-new-meter", log.Fields{"oldMeter": oldMeter, "newMeter": newMeter, "logical-device-id": agent.logicalDeviceID})
return nil
}
diff --git a/rw_core/core/device/logical_agent_port.go b/rw_core/core/device/logical_agent_port.go
index 05a5108..c82ada0 100644
--- a/rw_core/core/device/logical_agent_port.go
+++ b/rw_core/core/device/logical_agent_port.go
@@ -172,10 +172,10 @@
newPort.OfpPort = &newOfpPort
if state == voltha.OperStatus_ACTIVE {
- newOfpPort.Config = newOfpPort.Config & ^uint32(ofp.OfpPortConfig_OFPPC_PORT_DOWN)
+ newOfpPort.Config &= ^uint32(ofp.OfpPortConfig_OFPPC_PORT_DOWN)
newOfpPort.State = uint32(ofp.OfpPortState_OFPPS_LIVE)
} else {
- newOfpPort.Config = newOfpPort.Config | uint32(ofp.OfpPortConfig_OFPPC_PORT_DOWN)
+ newOfpPort.Config |= uint32(ofp.OfpPortConfig_OFPPC_PORT_DOWN)
newOfpPort.State = uint32(ofp.OfpPortState_OFPPS_LINK_DOWN)
}
return &newPort
@@ -272,7 +272,7 @@
newOfpPort := *oldPort.OfpPort
newPort.OfpPort = &newOfpPort
- newOfpPort.Config = newOfpPort.Config & ^uint32(ofp.OfpPortConfig_OFPPC_PORT_DOWN)
+ newOfpPort.Config &= ^uint32(ofp.OfpPortConfig_OFPPC_PORT_DOWN)
if err := portHandle.Update(ctx, &newPort); err != nil {
return err
}
diff --git a/rw_core/core/device/manager.go b/rw_core/core/device/manager.go
index 9982611..b3fa7b7 100755
--- a/rw_core/core/device/manager.go
+++ b/rw_core/core/device/manager.go
@@ -612,12 +612,11 @@
return err
}
logger.Debugw(ctx, "successfully-loaded-parent-and-children", log.Fields{"device-id": deviceID})
- } else {
+ } else if device.ParentId != "" {
// Scenario B - use the parentId of that device (root device) to trigger the loading
- if device.ParentId != "" {
- return dMgr.load(ctx, device.ParentId)
- }
+ return dMgr.load(ctx, device.ParentId)
}
+
return nil
}
@@ -633,7 +632,13 @@
// trigger loading the devices along with their children and parent in memory
func (dMgr *Manager) ReconcileDevices(ctx context.Context, ids *voltha.IDs) (*empty.Empty, error) {
ctx = utils.WithRPCMetadataContext(ctx, "ReconcileDevices")
- logger.Debugw(ctx, "reconcile-devices", log.Fields{"num-devices": len(ids.Items)})
+
+ numDevices := 0
+ if ids != nil {
+ numDevices = len(ids.Items)
+ }
+
+ logger.Debugw(ctx, "reconcile-devices", log.Fields{"num-devices": numDevices})
if ids != nil && len(ids.Items) != 0 {
toReconcile := len(ids.Items)
reconciled := 0
diff --git a/rw_core/core/device/state/transitions.go b/rw_core/core/device/state/transitions.go
index c3bd372..103697f 100644
--- a/rw_core/core/device/state/transitions.go
+++ b/rw_core/core/device/state/transitions.go
@@ -431,14 +431,14 @@
var tempHandler []transitionHandler
var m *match
bestMatch := &match{}
- for _, aTransition := range tMap.transitions {
+ for i := range tMap.transitions {
// consider transition only if it matches deviceType or is a wild card - any
- if aTransition.deviceType != deviceType && aTransition.deviceType != any {
+ if tMap.transitions[i].deviceType != deviceType && tMap.transitions[i].deviceType != any {
continue
}
- tempHandler, m = getHandler(pState, cState, &aTransition)
+ tempHandler, m = getHandler(pState, cState, &tMap.transitions[i])
if tempHandler != nil {
- if m.isExactMatch() && aTransition.deviceType == deviceType {
+ if m.isExactMatch() && tMap.transitions[i].deviceType == deviceType {
return tempHandler
} else if m.isExactMatch() || m.isBetterMatch(bestMatch) {
currentMatch = tempHandler
diff --git a/rw_core/core/kv.go b/rw_core/core/kv.go
index 84f707b..631e82a 100644
--- a/rw_core/core/kv.go
+++ b/rw_core/core/kv.go
@@ -31,8 +31,7 @@
func newKVClient(ctx context.Context, storeType string, address string, timeout time.Duration) (kvstore.Client, error) {
logger.Infow(ctx, "kv-store-type", log.Fields{"store": storeType})
- switch storeType {
- case "etcd":
+ if storeType == "etcd" {
return kvstore.NewEtcdClient(ctx, address, timeout, log.FatalLevel)
}
return nil, errors.New("unsupported-kv-store")
diff --git a/rw_core/flowdecomposition/flow_decomposer_test.go b/rw_core/flowdecomposition/flow_decomposer_test.go
index 88e6c03..7ea80fc 100644
--- a/rw_core/flowdecomposition/flow_decomposer_test.go
+++ b/rw_core/flowdecomposition/flow_decomposer_test.go
@@ -332,7 +332,7 @@
fa := &fu.FlowArgs{
MatchFields: []*ofp.OfpOxmOfbField{
fu.InPort(2),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
},
Actions: []*ofp.OfpAction{
fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101)),
@@ -348,7 +348,7 @@
fa = &fu.FlowArgs{
MatchFields: []*ofp.OfpOxmOfbField{
fu.InPort(2),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
},
Actions: []*ofp.OfpAction{
fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 102)),
@@ -364,7 +364,7 @@
fa = &fu.FlowArgs{
MatchFields: []*ofp.OfpOxmOfbField{
fu.InPort(2),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
},
Actions: []*ofp.OfpAction{
fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 103)),
@@ -380,7 +380,7 @@
fa = &fu.FlowArgs{
MatchFields: []*ofp.OfpOxmOfbField{
fu.InPort(2),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
},
Actions: []*ofp.OfpAction{
fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 104)),
@@ -533,7 +533,7 @@
KV: fu.OfpFlowModArgs{"priority": 1000},
MatchFields: []*ofp.OfpOxmOfbField{
fu.InPort(1),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
fu.EthType(0x888e),
},
Actions: []*ofp.OfpAction{
@@ -578,7 +578,7 @@
fu.InPort(2),
fu.TunnelId(uint64(1)),
fu.EthType(0x888e),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
},
Actions: []*ofp.OfpAction{
fu.PushVlan(0x8100),
@@ -837,7 +837,7 @@
KV: fu.OfpFlowModArgs{"priority": 5000, "table_id": 0},
MatchFields: []*ofp.OfpOxmOfbField{
fu.InPort(1),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
fu.VlanPcp(0),
},
Actions: []*ofp.OfpAction{
@@ -892,7 +892,7 @@
MatchFields: []*ofp.OfpOxmOfbField{
fu.InPort(2),
fu.TunnelId(uint64(1)),
- fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+ fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT)),
fu.VlanPcp(0),
},
Actions: []*ofp.OfpAction{
@@ -965,7 +965,7 @@
fu.VlanPcp(0),
},
Actions: []*ofp.OfpAction{
- fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0)),
+ fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT))),
fu.Output(1),
},
}
@@ -1034,7 +1034,7 @@
fu.VlanPcp(0),
},
Actions: []*ofp.OfpAction{
- fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0)),
+ fu.SetField(fu.VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT))),
fu.Output(2),
},
}
diff --git a/rw_core/main.go b/rw_core/main.go
index a3af416..38f8bd8 100644
--- a/rw_core/main.go
+++ b/rw_core/main.go
@@ -145,7 +145,12 @@
if err != nil {
logger.Warnw(ctx, "unable-to-initialize-tracing-and-log-correlation-module", log.Fields{"error": err})
} else {
- defer closer.Close()
+ defer func() {
+ err = closer.Close()
+ if err != nil {
+ logger.Errorw(ctx, "failed-to-close-trace-closer", log.Fields{"error": err})
+ }
+ }()
}
// create and start the core
diff --git a/rw_core/route/device_route.go b/rw_core/route/device_route.go
index 4e5854b..b1f78c2 100644
--- a/rw_core/route/device_route.go
+++ b/rw_core/route/device_route.go
@@ -400,13 +400,11 @@
routes = append(routes, path[1])
return routes, nil
}
- } else {
+ } else if ingress != 0 && routeLink.Ingress == ingress {
// Here we use the first route whose ingress port matches the ingress input parameter
- if ingress != 0 && routeLink.Ingress == ingress {
- routes = append(routes, path[0])
- routes = append(routes, Hop{})
- return routes, nil
- }
+ routes = append(routes, path[0])
+ routes = append(routes, Hop{})
+ return routes, nil
}
}
return routes, fmt.Errorf("no half route found for ingress port %d, egress port %d and nni as egress %t", ingress, egress, nniAsEgress)
diff --git a/rw_core/route/device_route_test.go b/rw_core/route/device_route_test.go
index 722c9f5..04623db 100644
--- a/rw_core/route/device_route_test.go
+++ b/rw_core/route/device_route_test.go
@@ -261,8 +261,8 @@
numUniPerOnu := 4
done := make(chan struct{})
- fmt.Println(fmt.Sprintf("Test: Computing all routes. LogicalPorts:%d, NNI:%d, Pon/OLT:%d, ONU/Pon:%d, Uni/Onu:%d",
- numNNIPort*numPonPortOnOlt*numOnuPerOltPonPort*numUniPerOnu, numNNIPort, numPonPortOnOlt, numOnuPerOltPonPort, numUniPerOnu))
+ fmt.Printf("Test: Computing all routes. LogicalPorts:%d, NNI:%d, Pon/OLT:%d, ONU/Pon:%d, Uni/Onu:%d\n",
+ numNNIPort*numPonPortOnOlt*numOnuPerOltPonPort*numUniPerOnu, numNNIPort, numPonPortOnOlt, numOnuPerOltPonPort, numUniPerOnu)
// Create all the devices and logical device before computing the routes in one go
ld := &voltha.LogicalDevice{Id: logicalDeviceID}
@@ -297,7 +297,12 @@
for _, port := range ldMgr.ports {
assert.Equal(t, port.RootPort, ldMgr.deviceRoutes.IsRootPort(port.OfpPort.PortNo))
}
- fmt.Println(fmt.Sprintf("Total Time:%dms, Total Routes:%d NumGetDeviceInvoked:%d", time.Since(start)/time.Millisecond, len(ldMgr.deviceRoutes.Routes), onuMgr.numGetDeviceInvoked))
+ fmt.Printf(
+ "Total Time:%dms, Total Routes:%d NumGetDeviceInvoked:%d\n",
+ time.Since(start)/time.Millisecond,
+ len(ldMgr.deviceRoutes.Routes),
+ onuMgr.numGetDeviceInvoked,
+ )
}
func TestDeviceRoutes_AddPort(t *testing.T) {
@@ -307,8 +312,8 @@
numUniPerOnu := 4
done := make(chan struct{})
- fmt.Println(fmt.Sprintf("Test: Computing all routes. LogicalPorts:%d, NNI:%d, Pon/OLT:%d, ONU/Pon:%d, Uni/Onu:%d",
- numNNIPort*numPonPortOnOlt*numOnuPerOltPonPort*numUniPerOnu, numNNIPort, numPonPortOnOlt, numOnuPerOltPonPort, numUniPerOnu))
+ fmt.Printf("Test: Computing all routes. LogicalPorts:%d, NNI:%d, Pon/OLT:%d, ONU/Pon:%d, Uni/Onu:%d\n",
+ numNNIPort*numPonPortOnOlt*numOnuPerOltPonPort*numUniPerOnu, numNNIPort, numPonPortOnOlt, numOnuPerOltPonPort, numUniPerOnu)
start := time.Now()
// Create all the devices and logical device before computing the routes in one go
@@ -330,7 +335,8 @@
close(oltMgrChnl)
close(ldMgrChnl)
- ldMgr.deviceRoutes.Print(ctx)
+ err := ldMgr.deviceRoutes.Print(ctx)
+ assert.NoError(t, err)
// Validate the routes are up to date
assert.True(t, ldMgr.deviceRoutes.isUpToDate(ldMgr.ports))
@@ -343,7 +349,12 @@
assert.Equal(t, port.RootPort, ldMgr.deviceRoutes.IsRootPort(port.OfpPort.PortNo))
}
- fmt.Println(fmt.Sprintf("Total Time:%dms, Total Routes:%d NumGetDeviceInvoked:%d", time.Since(start)/time.Millisecond, len(ldMgr.deviceRoutes.Routes), onuMgr.numGetDeviceInvoked))
+ fmt.Printf(
+ "Total Time:%dms, Total Routes:%d NumGetDeviceInvoked:%d\n",
+ time.Since(start)/time.Millisecond,
+ len(ldMgr.deviceRoutes.Routes),
+ onuMgr.numGetDeviceInvoked,
+ )
}
func TestDeviceRoutes_compareRoutesGeneration(t *testing.T) {
@@ -353,8 +364,8 @@
numUniPerOnu := 4
done := make(chan struct{})
- fmt.Println(fmt.Sprintf("Test: Computing all routes. LogicalPorts:%d, NNI:%d, Pon/OLT:%d, ONU/Pon:%d, Uni/Onu:%d",
- numNNIPort*numPonPortOnOlt*numOnuPerOltPonPort*numUniPerOnu, numNNIPort, numPonPortOnOlt, numOnuPerOltPonPort, numUniPerOnu))
+ fmt.Printf("Test: Computing all routes. LogicalPorts:%d, NNI:%d, Pon/OLT:%d, ONU/Pon:%d, Uni/Onu:%d\n",
+ numNNIPort*numPonPortOnOlt*numOnuPerOltPonPort*numUniPerOnu, numNNIPort, numPonPortOnOlt, numOnuPerOltPonPort, numUniPerOnu)
// Create all the devices and logical device before computing the routes in one go
ld1 := &voltha.LogicalDevice{Id: logicalDeviceID}
@@ -472,7 +483,7 @@
assert.Equal(t, ingressNos[j], reverseRoute[i].Egress)
}
- fmt.Println(fmt.Sprintf("Reverse of %d hops successful.", numRoutes))
+ fmt.Printf("Reverse of %d hops successful.\n", numRoutes)
reverseOfReverse := getReverseRoute(reverseRoute)
assert.Equal(t, route, reverseOfReverse)
diff --git a/rw_core/utils/id.go b/rw_core/utils/id.go
index 862b909..3bf6d19 100644
--- a/rw_core/utils/id.go
+++ b/rw_core/utils/id.go
@@ -17,9 +17,10 @@
package utils
import (
+ cryptoRand "crypto/rand"
+ "encoding/binary"
"errors"
"fmt"
- "math/rand"
"strconv"
"github.com/google/uuid"
@@ -36,9 +37,13 @@
}
// CreateLogicalPortID produces a random port ID for a logical device.
-func CreateLogicalPortID() uint32 {
- // A logical port is a uint32
- return rand.Uint32()
+func CreateLogicalPortID() (v uint64, err error) {
+ err = binary.Read(cryptoRand.Reader, binary.BigEndian, &v)
+ if err != nil {
+ return v, err
+ }
+
+ return v, nil
}
// CreateDataPathID creates uint64 pathid from string pathid