Skip to content
Permalink
Browse files

Merge pull request #2662 from telstra/kbps

Fixed incorrect converting meter rate/burstsize from packets to kilobits
  • Loading branch information...
nikitacherevko committed Aug 12, 2019
2 parents 198c7ea + fb762b1 commit 331287948c476d4995f42b0f0061a70483c64b62
@@ -174,7 +174,6 @@
public static final int FLOW_PRIORITY = FlowModUtils.PRIORITY_HIGH;
public static final int DEFAULT_FLOW_PRIORITY = FLOW_PRIORITY - 1;
public static final int BDF_DEFAULT_PORT = 3784;
public static final int MIN_RATE_IN_KBPS = 64;
public static final int ROUND_TRIP_LATENCY_GROUP_ID = 1;
public static final MacAddress STUB_VXLAN_ETH_DST_MAC = MacAddress.of(0xFFFFFFEDCBA2L);
public static final IPv4Address STUB_VXLAN_IPV4_SRC = IPv4Address.of("127.0.0.1");
@@ -388,7 +387,7 @@ public long installIngressFlow(DatapathId dpid, String flowId, Long cookie, int
OFFactory ofFactory = sw.getOFFactory();

// build meter instruction
OFInstructionMeter meter = buildMeterInstruction(meterId, sw, ofFactory, actionList);
OFInstructionMeter meter = buildMeterInstruction(meterId, sw, actionList);

// output action based on encap scheme
actionList.addAll(inputVlanTypeToOfActionList(ofFactory, transitTunnelId, outputVlanType,
@@ -495,7 +494,7 @@ public long installOneSwitchFlow(DatapathId dpid, String flowId, Long cookie, in


// build meter instruction
OFInstructionMeter meter = buildMeterInstruction(meterId, sw, ofFactory, actionList);
OFInstructionMeter meter = buildMeterInstruction(meterId, sw, actionList);

// output action based on encap scheme
actionList.addAll(pushSchemeOutputVlanTypeToOfActionList(ofFactory, outputVlanId, outputVlanType));
@@ -571,7 +570,7 @@ public long installOneSwitchFlow(DatapathId dpid, String flowId, Long cookie, in

private OFInstructionMeter buildMeterInstructionForBroadcastRule(IOFSwitch sw, ArrayList<OFAction> actionList) {
return buildMeterInstruction(createMeterIdForDefaultRule(VERIFICATION_BROADCAST_RULE_COOKIE).getValue(),
sw, sw.getOFFactory(), actionList);
sw, actionList);
}

private ArrayList<OFAction> prepareActionListForUnicastRule(IOFSwitch sw) {
@@ -582,12 +581,12 @@ private OFInstructionMeter buildMeterInstructionForBroadcastRule(IOFSwitch sw, A

private OFInstructionMeter buildMeterInstructionForUnicastRule(IOFSwitch sw, ArrayList<OFAction> actionList) {
return buildMeterInstruction(createMeterIdForDefaultRule(VERIFICATION_UNICAST_RULE_COOKIE).getValue(),
sw, sw.getOFFactory(), actionList);
sw, actionList);
}

private OFInstructionMeter buildMeterInstructionForUnicastVxlanRule(IOFSwitch sw, ArrayList<OFAction> actionList) {
return buildMeterInstruction(createMeterIdForDefaultRule(VERIFICATION_UNICAST_VXLAN_RULE_COOKIE).getValue(),
sw, sw.getOFFactory(), actionList);
sw, actionList);
}

/**
@@ -707,7 +706,7 @@ public void installMeterForFlow(DatapathId dpid, long bandwidth, final long mete
.collect(Collectors.toSet());
installMeter(sw, flags, bandwidth, burstSize, meterId);
} else {
throw new InvalidMeterIdException(dpid, "Meter id must be positive.");
throw new InvalidMeterIdException(dpid, format("Meter id must be greater than %d.", MIN_FLOW_METER_ID));
}
}

@@ -1857,7 +1856,14 @@ public InetAddress getSwitchIpAddress(IOFSwitch sw) {

@VisibleForTesting
OFInstructionMeter installMeterForDefaultRule(IOFSwitch sw, long meterId, long ratePkts,
ArrayList<OFAction> actionList) {
List<OFAction> actionList) {
return installOrReinstallMeter(sw, meterId, ratePkts, config.getDiscoPacketSize(),
config.getSystemMeterBurstSizeInPackets(), actionList);
}

private OFInstructionMeter installOrReinstallMeter(IOFSwitch sw, long meterId, long rateInPackets,
long packetSizeInBytes, long burstSizeInPackets,
List<OFAction> actionList) {
OFMeterConfig meterConfig;
try {
meterConfig = getMeter(sw.getId(), meterId);
@@ -1880,19 +1886,19 @@ OFInstructionMeter installMeterForDefaultRule(IOFSwitch sw, long meterId, long r
if (featureDetectorService.detectSwitch(sw).contains(Feature.PKTPS_FLAG)) {
flags = ImmutableSet.of(OFMeterFlags.PKTPS, OFMeterFlags.STATS, OFMeterFlags.BURST);
// With PKTPS flag rate and burst size is in packets
rate = ratePkts;
burstSize = config.getSystemMeterBurstSizeInPackets();
rate = rateInPackets;
burstSize = burstSizeInPackets;
} else {
flags = ImmutableSet.of(OFMeterFlags.KBPS, OFMeterFlags.STATS, OFMeterFlags.BURST);
// With KBPS flag rate and burst size is in Kbits
rate = Math.max(MIN_RATE_IN_KBPS, (ratePkts * config.getDiscoPacketSize()) / 1024L);
burstSize = config.getSystemMeterBurstSizeInPackets() * config.getDiscoPacketSize() / 1024L;
rate = Meter.convertRateToKiloBits(rateInPackets, packetSizeInBytes);
burstSize = Meter.convertBurstSizeToKiloBits(burstSizeInPackets, packetSizeInBytes);
}

if (meterBandDrop != null && meterBandDrop.getRate() == rate
&& CollectionUtils.isEqualCollection(meterConfig.getFlags(), flags)) {
logger.debug("Meter {} won't be reinstalled on switch {}. It already exists", meterId, sw.getId());
return buildMeterInstruction(meterId, sw, sw.getOFFactory(), actionList);
return buildMeterInstruction(meterId, sw, actionList);
}

if (meterBandDrop != null) {
@@ -1908,20 +1914,19 @@ OFInstructionMeter installMeterForDefaultRule(IOFSwitch sw, long meterId, long r
return null;
}

return buildMeterInstruction(meterId, sw, sw.getOFFactory(), actionList);
return buildMeterInstruction(meterId, sw, actionList);
}

/**
* Creates meter instruction for OF versions 1.3 and 1.4 or adds meter to actions.
*
* @param meterId meter to be installed.
* @param sw switch information.
* @param ofFactory OF factory for the switch.
* @param actionList actions for the flow.
* @return built {@link OFInstructionMeter} for OF 1.3 and 1.4, otherwise returns null.
*/
private OFInstructionMeter buildMeterInstruction(long meterId, IOFSwitch sw, OFFactory ofFactory,
List<OFAction> actionList) {
private OFInstructionMeter buildMeterInstruction(long meterId, IOFSwitch sw, List<OFAction> actionList) {
OFFactory ofFactory = sw.getOFFactory();
OFInstructionMeter meterInstruction = null;
if (meterId != 0L && (config.isOvsMetersEnabled() || !isOvs(sw))) {
if (ofFactory.getVersion().compareTo(OF_12) <= 0) {
@@ -1183,7 +1183,7 @@ public void shouldInstallMeterWithKbpsFlag() throws Exception {
contains(OFMeterFlags.KBPS, OFMeterFlags.STATS, OFMeterFlags.BURST))));
for (OFMeterMod mod : actual) {
long expectedBurstSize =
config.getSystemMeterBurstSizeInPackets() * config.getDiscoPacketSize() / 1024L;
config.getSystemMeterBurstSizeInPackets() * config.getDiscoPacketSize() * 8 / 1024L;
assertThat(mod.getMeters(), everyItem(hasProperty("burstSize", is(expectedBurstSize))));
}
}
@@ -121,10 +121,10 @@ class MetersSpec extends HealthCheckSpecification {
def meters = northbound.getAllMeters(sw.dpId)
assert meters.meterEntries.size() == 2
assert meters.meterEntries.each {
assert it.rate == Math.max((long) (DISCO_PKT_RATE * DISCO_PKT_SIZE / 1024L), MIN_RATE_KBPS)
assert it.rate == Math.max((long) (DISCO_PKT_RATE * DISCO_PKT_SIZE * 8 / 1024L), MIN_RATE_KBPS)
}
//unable to use #getExpectedBurst. For Centects there's special burst due to KBPS
assert meters.meterEntries.every { it.burstSize == (long) ((DISCO_PKT_BURST * DISCO_PKT_SIZE) / 1024) }
assert meters.meterEntries.every { it.burstSize == (long) ((DISCO_PKT_BURST * DISCO_PKT_SIZE * 8) / 1024) }
assert meters.meterEntries.every(defaultMeters)
assert meters.meterEntries.every { ["KBPS", "BURST", "STATS"].containsAll(it.flags) }
assert meters.meterEntries.every { it.flags.size() == 3 }
@@ -26,6 +26,7 @@
private static final double MAX_NOVIFLOW_BURST_COEFFICIENT = 1.005;
private static final long MIN_CENTEC_SWITCH_BURST_SIZE = 1024L;
private static final long MAX_CENTEC_SWITCH_BURST_SIZE = 32000L;
public static final int MIN_RATE_IN_KBPS = 64;

private static final String[] METER_FLAGS = {"KBPS", "BURST", "STATS"};

@@ -64,6 +65,20 @@ public static long calculateBurstSize(long bandwidth, long flowMeterMinBurstSize
return Math.round(bandwidth * burstCoefficient);
}

/**
* Convert rate from packets to kilobits.
*/
public static long convertRateToKiloBits(long rateInPackets, long packetSizeInBytes) {
return Math.max(MIN_RATE_IN_KBPS, (rateInPackets * packetSizeInBytes * 8) / 1024L);
}

/**
* Convert burst size from packets to kilobits.
*/
public static long convertBurstSizeToKiloBits(long burstSizeInPackets, long packetSizeInBytes) {
return (burstSizeInPackets * packetSizeInBytes * 8) / 1024L;
}

/**
* Get meter flags as string array.
*/
@@ -29,4 +29,17 @@ public void calculateBurstSize() {
assertEquals(1105500, Meter.calculateBurstSize(1100000L, 1024L, 1.005, "NW000.0.0"));
assertEquals(1105500, Meter.calculateBurstSize(1100000L, 1024L, 1.05, "NW000.0.0"));
}

@Test
public void convertRateToKiloBitsTest() {
assertEquals(800, Meter.convertRateToKiloBits(100, 1024));
assertEquals(64, Meter.convertRateToKiloBits(1, 1));
assertEquals(64, Meter.convertRateToKiloBits(8, 16));
}

@Test
public void convertBurstSizeToKiloBitsTest() {
assertEquals(800, Meter.convertBurstSizeToKiloBits(100, 1024));
assertEquals(1, Meter.convertBurstSizeToKiloBits(8, 16));
}
}

0 comments on commit 3312879

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