From 915babd970a9b4f209deceb3c4973b7d1c9c0c12 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Wed, 26 Sep 2012 17:14:57 -0700 Subject: [PATCH] fix kvm traffic labels (guest traffic types on multiple networks don't work) Cloudstack seems to let you create guest traffic types on multiple physical networks. However, when I try this with KVM I end up always bridging to whatever device is used for guest.network.device. This pulls the traffic label (NicTO.getName()) and uses that bridge to ensure that we get on the correct physical network, rather than just always using the guest.network.device. This also changes the bridge naming scheme from cloudVirBr + vlanid to br + physicalinterface + "-" + vlanid. This is because we should be able to support the same vlan numbers per physical network, and the previous bridge name would not support this and collide. Signed-off-by: Edison Su --- .../kvm/resource/BridgeVifDriver.java | 41 ++++++++++--- .../resource/LibvirtComputingResource.java | 61 +++++++++++-------- scripts/vm/network/vnet/modifyvlan.sh | 31 +++------- 3 files changed, 79 insertions(+), 54 deletions(-) diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index cf4de095cf7..116c09d3807 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -85,11 +85,18 @@ public class BridgeVifDriver extends VifDriverBase { URI broadcastUri = nic.getBroadcastUri(); vlanId = broadcastUri.getHost(); } + String trafficLabel = nic.getName(); if (nic.getType() == Networks.TrafficType.Guest) { if (nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan && !vlanId.equalsIgnoreCase("untagged")) { - String brName = createVlanBr(vlanId, _pifs.get("private")); - intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType)); + if(trafficLabel != null || !trafficLabel.isEmpty()) { + s_logger.debug("creating a vlan dev and bridge for guest traffic per traffic label " + trafficLabel); + String brName = createVlanBr(vlanId, _pifs.get(trafficLabel)); + intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType)); + } else { + String brName = createVlanBr(vlanId, _pifs.get("private")); + intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType)); + } } else { intf.defBridgeNet(_bridges.get("guest"), null, nic.getMac(), getGuestNicModel(guestOsType)); } @@ -100,8 +107,14 @@ public class BridgeVifDriver extends VifDriverBase { } else if (nic.getType() == Networks.TrafficType.Public) { if (nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan && !vlanId.equalsIgnoreCase("untagged")) { - String brName = createVlanBr(vlanId, _pifs.get("public")); - intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType)); + if(trafficLabel != null || !trafficLabel.isEmpty()){ + s_logger.debug("creating a vlan dev and bridge for public traffic per traffic label " + trafficLabel); + String brName = createVlanBr(vlanId, _pifs.get(trafficLabel)); + intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType)); + } else { + String brName = createVlanBr(vlanId, _pifs.get("public")); + intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType)); + } } else { intf.defBridgeNet(_bridges.get("public"), null, nic.getMac(), getGuestNicModel(guestOsType)); } @@ -120,22 +133,32 @@ public class BridgeVifDriver extends VifDriverBase { // Nothing needed as libvirt cleans up tap interface from bridge. } - private String setVnetBrName(String vnetId) { - return "cloudVirBr" + vnetId; + private String setVnetBrName(String pifName, String vnetId) { + String brName = "br" + pifName + "-"+ vnetId; + String oldStyleBrName = "cloudVirBr" + vnetId; + + String cmdout = Script.runSimpleBashScript("brctl show | grep " + oldStyleBrName); + if (cmdout != null && cmdout.contains(oldStyleBrName)) { + s_logger.info("Using old style bridge name for vlan " + vnetId + " because existing bridge " + oldStyleBrName + " was found"); + brName = oldStyleBrName; + } + + return brName; } private String createVlanBr(String vlanId, String nic) throws InternalErrorException { - String brName = setVnetBrName(vlanId); - createVnet(vlanId, nic); + String brName = setVnetBrName(nic, vlanId); + createVnet(vlanId, nic, brName); return brName; } - private void createVnet(String vnetId, String pif) + private void createVnet(String vnetId, String pif, String brName) throws InternalErrorException { final Script command = new Script(_modifyVlanPath, _timeout, s_logger); command.add("-v", vnetId); command.add("-p", pif); + command.add("-b", brName); command.add("-o", "add"); final String result = command.execute(); diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 22b149f5323..f6a6494f3cf 100755 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -44,6 +44,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.regex.Pattern; +import java.util.regex.Matcher; import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -777,32 +779,32 @@ public class LibvirtComputingResource extends ServerResourceBase implements } private void getPifs() { - /* get pifs from bridge */ - String pubPif = null; - String privPif = null; - String vlan = null; - if (_publicBridgeName != null) { - pubPif = Script.runSimpleBashScript("brctl show | grep " - + _publicBridgeName + " | awk '{print $4}'"); - vlan = Script.runSimpleBashScript("ls /proc/net/vlan/" + pubPif); - if (vlan != null && !vlan.isEmpty()) { - pubPif = Script - .runSimpleBashScript("grep ^Device\\: /proc/net/vlan/" - + pubPif + " | awk {'print $2'}"); + /* gather all available bridges and find their pifs, so that we can match them against traffic labels later */ + String cmdout = Script.runSimpleBashScript("brctl show | tail -n +2 | grep -v \"^\\s\"|awk '{print $1}'|sed '{:q;N;s/\\n/%/g;t q}'"); + s_logger.debug("cmdout was " + cmdout); + List bridges = Arrays.asList(cmdout.split("%")); + for (String bridge : bridges) { + s_logger.debug("looking for pif for bridge " + bridge); + String pif = getPif(bridge); + if(_publicBridgeName != null && bridge.equals(_publicBridgeName)){ + _pifs.put("public", pif); + } else if (_guestBridgeName != null) { + _pifs.put("private", pif); } + _pifs.put(bridge, pif); } - if (_guestBridgeName != null) { - privPif = Script.runSimpleBashScript("brctl show | grep " - + _guestBridgeName + " | awk '{print $4}'"); - vlan = Script.runSimpleBashScript("ls /proc/net/vlan/" + privPif); - if (vlan != null && !vlan.isEmpty()) { - privPif = Script - .runSimpleBashScript("grep ^Device\\: /proc/net/vlan/" - + privPif + " | awk {'print $2'}"); - } + s_logger.debug("done looking for pifs, no more bridges"); + } + + private String getPif(String bridge) { + String pif = Script.runSimpleBashScript("brctl show | grep " + bridge + " | awk '{print $4}'"); + String vlan = Script.runSimpleBashScript("ls /proc/net/vlan/" + pif); + + if (vlan != null && !vlan.isEmpty()) { + pif = Script.runSimpleBashScript("grep ^Device\\: /proc/net/vlan/" + pif + " | awk {'print $2'}"); } - _pifs.put("private", privPif); - _pifs.put("public", pubPif); + + return pif; } private boolean checkNetwork(String networkName) { @@ -3914,7 +3916,18 @@ public class LibvirtComputingResource extends ServerResourceBase implements } private String getVnetIdFromBrName(String vnetBrName) { - return vnetBrName.replaceAll("cloudVirBr", ""); + if (vnetBrName.contains("cloudVirBr")) { + return vnetBrName.replaceAll("cloudVirBr", ""); + } else { + Pattern r = Pattern.compile("-(\\d+)$"); + Matcher m = r.matcher(vnetBrName); + if(m.group(1) != null || !m.group(1).isEmpty()) { + return m.group(1); + } else { + s_logger.debug("unable to get a vlan ID from name " + vnetBrName); + return ""; + } + } } private void cleanupVMNetworks(Connect conn, List nics) { diff --git a/scripts/vm/network/vnet/modifyvlan.sh b/scripts/vm/network/vnet/modifyvlan.sh index 33d697a47bd..5577825ea54 100755 --- a/scripts/vm/network/vnet/modifyvlan.sh +++ b/scripts/vm/network/vnet/modifyvlan.sh @@ -22,15 +22,14 @@ # set -x usage() { - printf "Usage: %s: -o (add | delete) -v -p \n" + printf "Usage: %s: -o (add | delete) -v -p -b \n" } -VIRBR=cloudVirBr addVlan() { local vlanId=$1 local pif=$2 local vlanDev=$pif.$vlanId - local vlanBr=$VIRBR$vlanId + local vlanBr=$3 vconfig set_name_type DEV_PLUS_VID_NO_PAD @@ -99,7 +98,7 @@ deleteVlan() { local vlanId=$1 local pif=$2 local vlanDev=$pif.$vlanId - local vlanBr=$VIRBR$vlanId + local vlanBr=$3 vconfig rem $vlanDev > /dev/null @@ -132,7 +131,7 @@ op= vlanId= option=$@ -while getopts 'o:v:p:' OPTION +while getopts 'o:v:p:b:' OPTION do case $OPTION in o) oflag=1 @@ -144,6 +143,9 @@ do p) pflag=1 pif="$OPTARG" ;; + b) bflag=1 + brName="$OPTARG" + ;; ?) usage exit 2 ;; @@ -151,7 +153,7 @@ do done # Check that all arguments were passed in -if [ "$oflag$vflag$pflag" != "111" ] +if [ "$oflag$vflag$pflag$bflag" != "1111" ] then usage exit 2 @@ -167,7 +169,7 @@ fi if [ "$op" == "add" ] then # Add the vlan - addVlan $vlanId $pif + addVlan $vlanId $pif $brName # If the add fails then return failure if [ $? -gt 0 ] @@ -178,22 +180,9 @@ else if [ "$op" == "delete" ] then # Delete the vlan - deleteVlan $vlanId $pif + deleteVlan $vlanId $pif $brName # Always exit with success exit 0 fi fi - - - - - - - - - - - - -