Skip to content
Permalink
Browse files

Merge branch 'release-1.41.0' into develop

  • Loading branch information...
surabujin committed Nov 6, 2019
2 parents 9d4f1f7 + ea8b3fe commit a7f83da0bd0549b08e171c0be9f6f25fa1df261f
@@ -1,5 +1,35 @@
# Changelog

## v1.41.0 (06/11/2019)

### Features:
- [#2243](https://github.com/telstra/open-kilda/pull/2243) Extend flow validation with meter validation. (Issue: [#1249](https://github.com/telstra/open-kilda/issues/1249)) [**floodlight**][**storm-topologies**]
- [#2845](https://github.com/telstra/open-kilda/pull/2845) add tests for v1/config API [**tests**]

### Bug Fixes:
- [#2887](https://github.com/telstra/open-kilda/pull/2887) Added noviflow virtual switch checks to FeatureDetectorService [**floodlight**]
- [#2898](https://github.com/telstra/open-kilda/pull/2898) Fix flow validation for Centec and E switches. [**storm-topologies**]
- [#2901](https://github.com/telstra/open-kilda/pull/2901) Fix flow validation for Accton switches. [**floodlight**][**storm-topologies**]

### Improvements:
- [#2880](https://github.com/telstra/open-kilda/pull/2880) improve checks for installed rules in vxlanFlowSpec [**tests**]
- [#2854](https://github.com/telstra/open-kilda/pull/2854) refactor "System takes isl time_unstable info into account while creating a flow" [**tests**]
- [#2663](https://github.com/telstra/open-kilda/pull/2663) Log message if ISL has negative cost (Issue: [#2319](https://github.com/telstra/open-kilda/issues/2319)) [**storm-topologies**]
- [#2891](https://github.com/telstra/open-kilda/pull/2891) Increase PortHistorySpec stability [**tests**]
- [#2482](https://github.com/telstra/open-kilda/pull/2482) Move flow validation to Nbworker topology. (Issue: [#1442](https://github.com/telstra/open-kilda/issues/1442)) [**floodlight**][**northbound**][**storm-topologies**]
- [#2680](https://github.com/telstra/open-kilda/pull/2680) Extend network topology dashboard logger (Issue: [#2659](https://github.com/telstra/open-kilda/issues/2659)) [**floodlight**][**storm-topologies**]
- [#2299](https://github.com/telstra/open-kilda/pull/2299) Make meter modify logic using the H&S approach. (Issue: [#2298](https://github.com/telstra/open-kilda/issues/2298)) [**floodlight**][**storm-topologies**]
- [#2877](https://github.com/telstra/open-kilda/pull/2877) Fix minor sonar issues [**floodlight**][**storm-topologies**]
- [#2846](https://github.com/telstra/open-kilda/pull/2846) add test System does not create a flow when bandwidth is not the same on the ISL [**tests**]


For the complete list of changes, check out [the commit log](https://github.com/telstra/open-kilda/compare/v1.40.0...v1.41.0).

### Affected Components:
flow, nbworker, nb, network, fl, flow-hs

---

## v1.40.0 (28/10/2019)

### Features:
@@ -1358,7 +1358,7 @@ private void dumpMeters(SwitchId switchId, String correlationId, String replyToT
producerService.sendMessageAndTrack(replyToTopic, correlationId, infoMessage);
} catch (UnsupportedSwitchOperationException e) {
logger.info("Meters not supported: {}", switchId);
InfoMessage infoMessage = new InfoMessage(new SwitchMeterUnsupported(), timestamp, correlationId);
InfoMessage infoMessage = new InfoMessage(new SwitchMeterUnsupported(switchId), timestamp, correlationId);
producerService.sendMessageAndTrack(replyToTopic, correlationId, infoMessage);
} catch (SwitchNotFoundException e) {
logger.info("Dumping switch meters is unsuccessful. Switch {} not found", switchId);
@@ -29,7 +29,9 @@
private static final long MAX_CENTEC_SWITCH_BURST_SIZE = 32000L;
public static final int MIN_RATE_IN_KBPS = 64;

private static final int METER_BURST_SIZE_EQUALS_EPS = 1;
private static final int METER_BURST_SIZE_EQUALS_DELTA = 1;
private static final double E_SWITCH_METER_RATE_EQUALS_DELTA_COEFFICIENT = 0.01;
private static final double E_SWITCH_METER_BURST_SIZE_EQUALS_DELTA_COEFFICIENT = 0.01;

private static final String[] METER_KBPS_FLAGS = {"KBPS", "BURST", "STATS"};
private static final String[] METER_PKTPS_FLAGS = {"PKTPS", "BURST", "STATS"};
@@ -109,9 +111,28 @@ public static long convertBurstSizeToKiloBits(long burstSizeInPackets, long pack
}

/**
* Compare burst size.
* Returns true if the actual and expected rates are equal to each other and false otherwise.
*/
public static boolean equalsBurstSize(long first, long second) {
return Math.abs(first - second) <= METER_BURST_SIZE_EQUALS_EPS;
public static boolean equalsRate(long actual, long expected, boolean isESwitch) {
// E-switches have a bug when installing the rate and burst size.
// Such switch sets the rate different from the rate that was sent to it.
// Therefore, we compare actual and expected values ​​using the delta coefficient.
if (isESwitch) {
return Math.abs(actual - expected) <= expected * E_SWITCH_METER_RATE_EQUALS_DELTA_COEFFICIENT;
}
return actual == expected;
}

/**
* Returns true if the actual and expected burst sizes are equal to each other and false otherwise.
*/
public static boolean equalsBurstSize(long actual, long expected, boolean isESwitch) {
// E-switches have a bug when installing the rate and burst size.
// Such switch sets the burst size different from the burst size that was sent to it.
// Therefore, we compare actual and expected values ​​using the delta coefficient.
if (isESwitch) {
return Math.abs(actual - expected) <= expected * E_SWITCH_METER_BURST_SIZE_EQUALS_DELTA_COEFFICIENT;
}
return Math.abs(actual - expected) <= METER_BURST_SIZE_EQUALS_DELTA;
}
}
@@ -15,5 +15,22 @@

package org.openkilda.messaging.info.meter;

import org.openkilda.model.SwitchId;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Builder;
import lombok.Value;

@Value
@Builder
public class SwitchMeterUnsupported extends SwitchMeterData {

@JsonProperty(value = "switch_id")
private SwitchId switchId;

@JsonCreator
public SwitchMeterUnsupported(@JsonProperty(value = "switch_id") SwitchId switchId) {
this.switchId = switchId;
}
}
@@ -76,7 +76,7 @@ private SimpleSwitchRule buildIngressSimpleSwitchRule(Flow flow, FlowPath flowPa
.meterId(flowPath.getMeterId() != null ? flowPath.getMeterId().getValue() : null)
.meterRate(flow.getBandwidth())
.meterBurstSize(Meter.calculateBurstSize(flow.getBandwidth(), flowMeterMinBurstSizeInKbits,
flowMeterBurstCoefficient, flow.getSrcSwitch().getDescription()))
flowMeterBurstCoefficient, flowPath.getSrcSwitch().getDescription()))
.meterFlags(Meter.getMeterKbpsFlags())
.build();

@@ -74,4 +74,13 @@ public void declareOutputFields(OutputFieldsDeclarer declarer) {
super.declareOutputFields(declarer);
declarer.declareStream(StreamType.TO_SPEAKER.name(), MessageKafkaTranslator.STREAM_FIELDS);
}

@Override
protected void unhandledInput(Tuple input) {
if (log.isDebugEnabled()) {
log.trace("Received a response from {} for non-pending task {}: {}", input.getSourceComponent(),
input.getStringByField(MessageKafkaTranslator.FIELD_ID_KEY),
input.getValueByField(MessageKafkaTranslator.FIELD_ID_PAYLOAD));
}
}
}
@@ -39,6 +39,7 @@
import org.openkilda.persistence.PersistenceManager;
import org.openkilda.wfm.error.FlowNotFoundException;
import org.openkilda.wfm.error.IllegalFlowStateException;
import org.openkilda.wfm.error.SwitchNotFoundException;
import org.openkilda.wfm.share.flow.resources.FlowResourcesConfig;
import org.openkilda.wfm.topology.nbworker.bolts.FlowValidationHubCarrier;
import org.openkilda.wfm.topology.nbworker.fsm.FlowValidationFsm.FlowValidationEvent;
@@ -185,6 +186,9 @@ protected void validateFlow(FlowValidationState from, FlowValidationState to,
} catch (FlowNotFoundException e) {
log.error("Key: {}; Flow {} not found during flow validation", key, flowId, e);
sendException(e.getMessage(), "Flow validation operation in FlowValidationFsm", ErrorType.NOT_FOUND);
} catch (SwitchNotFoundException e) {
log.error("Key: {}; {}", key, e.getMessage(), e);
sendException(e.getMessage(), "Flow validation operation in FlowValidationFsm", ErrorType.NOT_FOUND);
} catch (Exception e) {
log.error("Key: {}; {}", key, e.getMessage(), e);
sendException(e.getMessage(), "Flow validation operation in FlowValidationFsm", ErrorType.INTERNAL_ERROR);
@@ -22,6 +22,7 @@
import org.openkilda.messaging.info.InfoData;
import org.openkilda.messaging.info.InfoMessage;
import org.openkilda.messaging.info.meter.SwitchMeterEntries;
import org.openkilda.messaging.info.meter.SwitchMeterUnsupported;
import org.openkilda.messaging.info.rule.SwitchFlowEntries;
import org.openkilda.messaging.nbtopology.request.FlowValidationRequest;
import org.openkilda.persistence.PersistenceManager;
@@ -35,6 +36,7 @@
import org.squirrelframework.foundation.fsm.StateMachineBuilder;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -80,6 +82,13 @@ public void handleAsyncResponse(String key, Message message) {
fsm.fire(FlowValidationEvent.RULES_RECEIVED, data);
} else if (data instanceof SwitchMeterEntries) {
fsm.fire(FlowValidationEvent.METERS_RECEIVED, data);
} else if (data instanceof SwitchMeterUnsupported) {
SwitchMeterUnsupported meterUnsupported = (SwitchMeterUnsupported) data;
log.info("Key: {}; Meters unsupported for switch '{};", key, meterUnsupported.getSwitchId());
fsm.fire(FlowValidationEvent.METERS_RECEIVED, SwitchMeterEntries.builder()
.switchId(meterUnsupported.getSwitchId())
.meterEntries(Collections.emptyList())
.build());
} else {
log.warn("Key: {}; Unhandled message {}", key, message);
}
@@ -23,13 +23,15 @@
import org.openkilda.model.Flow;
import org.openkilda.model.FlowPath;
import org.openkilda.model.FlowStatus;
import org.openkilda.model.Meter;
import org.openkilda.model.Switch;
import org.openkilda.model.SwitchId;
import org.openkilda.persistence.PersistenceManager;
import org.openkilda.persistence.repositories.FlowRepository;
import org.openkilda.persistence.repositories.SwitchRepository;
import org.openkilda.wfm.error.FlowNotFoundException;
import org.openkilda.wfm.error.IllegalFlowStateException;
import org.openkilda.wfm.error.SwitchNotFoundException;
import org.openkilda.wfm.share.flow.resources.EncapsulationResources;
import org.openkilda.wfm.share.flow.resources.FlowResourcesConfig;
import org.openkilda.wfm.share.flow.resources.FlowResourcesManager;
@@ -92,7 +94,7 @@ public void checkFlowStatus(String flowId) throws FlowNotFoundException, Illegal
*/
public List<FlowValidationResponse> validateFlow(String flowId, List<SwitchFlowEntries> switchFlowEntries,
List<SwitchMeterEntries> switchMeterEntries)
throws FlowNotFoundException {
throws FlowNotFoundException, SwitchNotFoundException {

Map<SwitchId, List<SimpleSwitchRule>> switchRules = new HashMap<>();
int rulesCount = 0;
@@ -154,13 +156,15 @@ public void checkFlowStatus(String flowId) throws FlowNotFoundException, Illegal

private FlowValidationResponse compare(Map<SwitchId, List<SimpleSwitchRule>> rulesPerSwitch,
List<SimpleSwitchRule> rulesFromDb, String flowId,
int totalSwitchRules, int metersCount) {
int totalSwitchRules, int metersCount) throws SwitchNotFoundException {

List<PathDiscrepancyEntity> discrepancies = new ArrayList<>();
List<Long> pktCounts = new ArrayList<>();
List<Long> byteCounts = new ArrayList<>();
rulesFromDb.forEach(simpleRule -> discrepancies.addAll(
findDiscrepancy(simpleRule, rulesPerSwitch.get(simpleRule.getSwitchId()), pktCounts, byteCounts)));
for (SimpleSwitchRule simpleRule : rulesFromDb) {
discrepancies.addAll(
findDiscrepancy(simpleRule, rulesPerSwitch.get(simpleRule.getSwitchId()), pktCounts, byteCounts));
}
int flowMetersCount = (int) rulesFromDb.stream().filter(rule -> rule.getMeterId() != null).count();

return FlowValidationResponse.builder()
@@ -197,7 +201,8 @@ private FlowValidationResponse compare(Map<SwitchId, List<SimpleSwitchRule>> rul
}

private List<PathDiscrepancyEntity> findDiscrepancy(SimpleSwitchRule expected, List<SimpleSwitchRule> actual,
List<Long> pktCounts, List<Long> byteCounts) {
List<Long> pktCounts, List<Long> byteCounts)
throws SwitchNotFoundException {
List<PathDiscrepancyEntity> discrepancies = new ArrayList<>();
SimpleSwitchRule matched = findMatched(expected, actual);

@@ -242,7 +247,8 @@ private SimpleSwitchRule findMatched(SimpleSwitchRule expected, List<SimpleSwitc
return matched;
}

private List<PathDiscrepancyEntity> getRuleDiscrepancies(SimpleSwitchRule expected, SimpleSwitchRule matched) {
private List<PathDiscrepancyEntity> getRuleDiscrepancies(SimpleSwitchRule expected, SimpleSwitchRule matched)
throws SwitchNotFoundException {
List<PathDiscrepancyEntity> discrepancies = new ArrayList<>();
if (matched.getCookie() != expected.getCookie()) {
discrepancies.add(new PathDiscrepancyEntity(expected.toString(), "cookie",
@@ -276,11 +282,17 @@ private SimpleSwitchRule findMatched(SimpleSwitchRule expected, List<SimpleSwitc
discrepancies.add(new PathDiscrepancyEntity(expected.toString(), "meterId",
String.valueOf(expected.getMeterId()), String.valueOf(matched.getMeterId())));
} else {
if (!Objects.equals(matched.getMeterRate(), expected.getMeterRate())) {

Switch sw = switchRepository.findById(expected.getSwitchId())
.orElseThrow(() -> new SwitchNotFoundException(expected.getSwitchId()));
boolean isESwitch =
Switch.isNoviflowESwitch(sw.getOfDescriptionManufacturer(), sw.getOfDescriptionHardware());

if (!equalsRate(matched.getMeterRate(), expected.getMeterRate(), isESwitch)) {
discrepancies.add(new PathDiscrepancyEntity(expected.toString(), "meterRate",
String.valueOf(expected.getMeterRate()), String.valueOf(matched.getMeterRate())));
}
if (!Objects.equals(matched.getMeterBurstSize(), expected.getMeterBurstSize())) {
if (!equalsBurstSize(matched.getMeterBurstSize(), expected.getMeterBurstSize(), isESwitch)) {
discrepancies.add(new PathDiscrepancyEntity(expected.toString(), "meterBurstSize",
String.valueOf(expected.getMeterBurstSize()), String.valueOf(matched.getMeterBurstSize())));
}
@@ -292,4 +304,20 @@ private SimpleSwitchRule findMatched(SimpleSwitchRule expected, List<SimpleSwitc
}
return discrepancies;
}

private boolean equalsRate(Long actual, Long expected, boolean isESwitch) {
if (actual == null || expected == null) {
return Objects.equals(actual, expected);
}

return Meter.equalsRate(actual, expected, isESwitch);
}

private boolean equalsBurstSize(Long actual, Long expected, boolean isESwitch) {
if (actual == null || expected == null) {
return Objects.equals(actual, expected);
}

return Meter.equalsBurstSize(actual, expected, isESwitch);
}
}
@@ -55,10 +55,6 @@

@Slf4j
public class ValidationServiceImpl implements ValidationService {
private static final int METER_BURST_SIZE_EQUALS_DELTA = 1;
private static final double E_SWITCH_METER_RATE_EQUALS_DELTA_COEFFICIENT = 0.01;
private static final double E_SWITCH_METER_BURST_SIZE_EQUALS_DELTA_COEFFICIENT = 0.01;

private FlowPathRepository flowPathRepository;
private final long flowMeterMinBurstSizeInKbits;
private final double flowMeterBurstCoefficient;
@@ -238,8 +234,8 @@ private ValidateMetersResult comparePresentedAndExpectedMeters(
continue;
}

if (equalsRate(presentedMeter.getRate(), expectedMeter.getRate(), isESwitch)
&& equalsBurstSize(presentedMeter.getBurstSize(), expectedMeter.getBurstSize(), isESwitch)
if (Meter.equalsRate(presentedMeter.getRate(), expectedMeter.getRate(), isESwitch)
&& Meter.equalsBurstSize(presentedMeter.getBurstSize(), expectedMeter.getBurstSize(), isESwitch)
&& Arrays.equals(presentedMeter.getFlags(), expectedMeter.getFlags())) {

properMeters.add(makeProperMeterEntry(
@@ -351,11 +347,11 @@ private MeterInfoEntry makeMisconfiguredMeterEntry(MeterEntry actualMeter, Simpl
MeterMisconfiguredInfoEntry actual = new MeterMisconfiguredInfoEntry();
MeterMisconfiguredInfoEntry expected = new MeterMisconfiguredInfoEntry();

if (!equalsRate(actualMeter.getRate(), expectedMeter.getRate(), isESwitch)) {
if (!Meter.equalsRate(actualMeter.getRate(), expectedMeter.getRate(), isESwitch)) {
actual.setRate(actualMeter.getRate());
expected.setRate(expectedMeter.getRate());
}
if (!equalsBurstSize(actualMeter.getBurstSize(), expectedMeter.getBurstSize(), isESwitch)) {
if (!Meter.equalsBurstSize(actualMeter.getBurstSize(), expectedMeter.getBurstSize(), isESwitch)) {
actual.setBurstSize(actualMeter.getBurstSize());
expected.setBurstSize(expectedMeter.getBurstSize());
}
@@ -379,24 +375,4 @@ private MeterInfoEntry makeMisconfiguredMeterEntry(MeterEntry actualMeter, Simpl
private Predicate<FlowPath> filterOutProtectedPathWithSrcEndpoint(SwitchId switchId) {
return path -> !(path.isProtected() && path.getSrcSwitch().getSwitchId().equals(switchId));
}

private boolean equalsRate(long actual, long expected, boolean isESwitch) {
// E-switches have a bug when installing the rate and burst size.
// Such switch sets the rate different from the rate that was sent to it.
// Therefore, we compare actual and expected values ​​using the delta coefficient.
if (isESwitch) {
return Math.abs(actual - expected) <= expected * E_SWITCH_METER_RATE_EQUALS_DELTA_COEFFICIENT;
}
return actual == expected;
}

private boolean equalsBurstSize(long actual, long expected, boolean isESwitch) {
// E-switches have a bug when installing the rate and burst size.
// Such switch sets the burst size different from the burst size that was sent to it.
// Therefore, we compare actual and expected values ​​using the delta coefficient.
if (isESwitch) {
return Math.abs(actual - expected) <= expected * E_SWITCH_METER_BURST_SIZE_EQUALS_DELTA_COEFFICIENT;
}
return Math.abs(actual - expected) <= METER_BURST_SIZE_EQUALS_DELTA;
}
}

0 comments on commit a7f83da

Please sign in to comment.
You can’t perform that action at this time.