You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2018/11/29 09:36:36 UTC

[cloudstack] branch master updated (5d7b33a -> ac9562a)

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git.


    from 5d7b33a  Fix null pointer exception when using ConfigKey.Scope.ManagementServer (#3062)
     add a84f7df  marvin: add missing default test data (#3055)
     add d425a40  sg: add secondary ips to the correct ipset based on ip family (#2990)
     add 29b8a9d  kvm: when untagged vxlan is used, use the default guest/public bridge (#3037)
     new ac9562a  Merge remote-tracking branch 'origin/4.11'

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../hypervisor/kvm/resource/BridgeVifDriver.java   | 19 ++++--
 .../kvm/resource/BridgeVifDriverTest.java          | 58 +++++++++++++++++
 scripts/vm/network/security_group.py               | 73 +++++++++++++++-------
 tools/marvin/marvin/config/test_data.py            |  6 ++
 4 files changed, 129 insertions(+), 27 deletions(-)
 create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java


[cloudstack] 01/01: Merge remote-tracking branch 'origin/4.11'

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit ac9562a4a13692aeb5d309bbffb278c836bb55e3
Merge: 5d7b33a 29b8a9d
Author: Rohit Yadav <ro...@shapeblue.com>
AuthorDate: Thu Nov 29 15:06:06 2018 +0530

    Merge remote-tracking branch 'origin/4.11'
    
    Signed-off-by: Rohit Yadav <ro...@shapeblue.com>

 .../hypervisor/kvm/resource/BridgeVifDriver.java   | 19 ++++--
 .../kvm/resource/BridgeVifDriverTest.java          | 58 +++++++++++++++++
 scripts/vm/network/security_group.py               | 73 +++++++++++++++-------
 tools/marvin/marvin/config/test_data.py            |  6 ++
 4 files changed, 129 insertions(+), 27 deletions(-)

diff --cc plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
index f6ca0a8,0000000..39b92c3
mode 100644,000000..100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
@@@ -1,453 -1,0 +1,460 @@@
 +/*
 + * Licensed to the Apache Software Foundation (ASF) under one
 + * or more contributor license agreements.  See the NOTICE file
 + * distributed with this work for additional information
 + * regarding copyright ownership.  The ASF licenses this file
 + * to you under the Apache License, Version 2.0 (the
 + * "License"); you may not use this file except in compliance
 + * with the License.  You may obtain a copy of the License at
 + *
 + *   http://www.apache.org/licenses/LICENSE-2.0
 + *
 + * Unless required by applicable law or agreed to in writing,
 + * software distributed under the License is distributed on an
 + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
 + * KIND, either express or implied.  See the License for the
 + * specific language governing permissions and limitations
 + * under the License.
 + */
 +
 +package com.cloud.hypervisor.kvm.resource;
 +
 +import java.io.File;
 +import java.util.ArrayList;
 +import java.util.List;
 +import java.util.Map;
 +import java.util.regex.Matcher;
 +import java.util.regex.Pattern;
 +
 +import javax.naming.ConfigurationException;
 +
 +import org.apache.log4j.Logger;
 +import org.libvirt.LibvirtException;
 +
 +import com.google.common.base.Strings;
 +
 +import com.cloud.agent.api.to.NicTO;
 +import com.cloud.exception.InternalErrorException;
 +import com.cloud.network.Networks;
 +import com.cloud.utils.NumbersUtil;
 +import com.cloud.utils.net.NetUtils;
 +import com.cloud.utils.script.OutputInterpreter;
 +import com.cloud.utils.script.Script;
 +
 +public class BridgeVifDriver extends VifDriverBase {
 +
 +    private static final Logger s_logger = Logger.getLogger(BridgeVifDriver.class);
 +    private int _timeout;
 +
 +    private final Object _vnetBridgeMonitor = new Object();
 +    private String _modifyVlanPath;
 +    private String _modifyVxlanPath;
 +    private String bridgeNameSchema;
 +    private Long libvirtVersion;
 +
 +    @Override
 +    public void configure(Map<String, Object> params) throws ConfigurationException {
 +
 +        super.configure(params);
 +
 +        getPifs();
 +
 +        // Set the domr scripts directory
 +        params.put("domr.scripts.dir", "scripts/network/domr/kvm");
 +
 +        String networkScriptsDir = (String)params.get("network.scripts.dir");
 +        if (networkScriptsDir == null) {
 +            networkScriptsDir = "scripts/vm/network/vnet";
 +        }
 +
 +        bridgeNameSchema = (String)params.get("network.bridge.name.schema");
 +
 +        String value = (String)params.get("scripts.timeout");
 +        _timeout = NumbersUtil.parseInt(value, 30 * 60) * 1000;
 +
 +        _modifyVlanPath = Script.findScript(networkScriptsDir, "modifyvlan.sh");
 +        if (_modifyVlanPath == null) {
 +            throw new ConfigurationException("Unable to find modifyvlan.sh");
 +        }
 +        _modifyVxlanPath = Script.findScript(networkScriptsDir, "modifyvxlan.sh");
 +        if (_modifyVxlanPath == null) {
 +            throw new ConfigurationException("Unable to find modifyvxlan.sh");
 +        }
 +
 +        libvirtVersion = (Long) params.get("libvirtVersion");
 +        if (libvirtVersion == null) {
 +            libvirtVersion = 0L;
 +        }
 +    }
 +
 +    public void getPifs() {
 +        final File dir = new File("/sys/devices/virtual/net");
 +        final File[] netdevs = dir.listFiles();
 +        final List<String> bridges = new ArrayList<String>();
 +        for (File netdev : netdevs) {
 +            final File isbridge = new File(netdev.getAbsolutePath() + "/bridge");
 +            final String netdevName = netdev.getName();
 +            s_logger.debug("looking in file " + netdev.getAbsolutePath() + "/bridge");
 +            if (isbridge.exists()) {
 +                s_logger.debug("Found bridge " + netdevName);
 +                bridges.add(netdevName);
 +            }
 +        }
 +
 +        String guestBridgeName = _libvirtComputingResource.getGuestBridgeName();
 +        String publicBridgeName = _libvirtComputingResource.getPublicBridgeName();
 +
 +        for (final String bridge : bridges) {
 +            s_logger.debug("looking for pif for bridge " + bridge);
 +            final String pif = getPif(bridge);
 +            if (_libvirtComputingResource.isPublicBridge(bridge)) {
 +                _pifs.put("public", pif);
 +            }
 +            if (guestBridgeName != null && bridge.equals(guestBridgeName)) {
 +                _pifs.put("private", pif);
 +            }
 +            _pifs.put(bridge, pif);
 +        }
 +
 +        // guest(private) creates bridges on a pif, if private bridge not found try pif direct
 +        // This addresses the unnecessary requirement of someone to create an unused bridge just for traffic label
 +        if (_pifs.get("private") == null) {
 +            s_logger.debug("guest(private) traffic label '" + guestBridgeName + "' not found as bridge, looking for physical interface");
 +            final File dev = new File("/sys/class/net/" + guestBridgeName);
 +            if (dev.exists()) {
 +                s_logger.debug("guest(private) traffic label '" + guestBridgeName + "' found as a physical device");
 +                _pifs.put("private", guestBridgeName);
 +            }
 +        }
 +
 +        // public creates bridges on a pif, if private bridge not found try pif direct
 +        // This addresses the unnecessary requirement of someone to create an unused bridge just for traffic label
 +        if (_pifs.get("public") == null) {
 +            s_logger.debug("public traffic label '" + publicBridgeName+ "' not found as bridge, looking for physical interface");
 +            final File dev = new File("/sys/class/net/" + publicBridgeName);
 +            if (dev.exists()) {
 +                s_logger.debug("public traffic label '" + publicBridgeName + "' found as a physical device");
 +                _pifs.put("public", publicBridgeName);
 +            }
 +        }
 +
 +        s_logger.debug("done looking for pifs, no more bridges");
 +    }
 +
 +    private String getPif(final String bridge) {
 +        String pif = matchPifFileInDirectory(bridge);
 +        final File vlanfile = new File("/proc/net/vlan/" + pif);
 +
 +        if (vlanfile.isFile()) {
 +            pif = Script.runSimpleBashScript("grep ^Device\\: /proc/net/vlan/" + pif + " | awk {'print $2'}");
 +        }
 +
 +        return pif;
 +    }
 +
 +    private String matchPifFileInDirectory(final String bridgeName) {
 +        final File brif = new File("/sys/devices/virtual/net/" + bridgeName + "/brif");
 +
 +        if (!brif.isDirectory()) {
 +            final File pif = new File("/sys/class/net/" + bridgeName);
 +            if (pif.isDirectory()) {
 +                // if bridgeName already refers to a pif, return it as-is
 +                return bridgeName;
 +            }
 +            s_logger.debug("failing to get physical interface from bridge " + bridgeName + ", does " + brif.getAbsolutePath() + "exist?");
 +            return "";
 +        }
 +
 +        final File[] interfaces = brif.listFiles();
 +
 +        for (File anInterface : interfaces) {
 +            final String fname = anInterface.getName();
 +            s_logger.debug("matchPifFileInDirectory: file name '" + fname + "'");
 +            if (isInterface(fname)) {
 +                return fname;
 +            }
 +        }
 +
 +        s_logger.debug("failing to get physical interface from bridge " + bridgeName + ", did not find an eth*, bond*, team*, vlan*, em*, p*p*, ens*, eno*, enp*, or enx* in " + brif.getAbsolutePath());
 +        return "";
 +    }
 +
 +    private static final String [] IF_NAME_PATTERNS = {
 +            "^eth",
 +            "^bond",
 +            "^vlan",
 +            "^vx",
 +            "^em",
 +            "^ens",
 +            "^eno",
 +            "^enp",
 +            "^team",
 +            "^enx",
 +            "^p\\d+p\\d+"
 +    };
 +
 +    /**
 +     * @param fname
 +     * @return
 +     */
 +    private static boolean isInterface(final String fname) {
 +        StringBuilder commonPattern = new StringBuilder();
 +        for (final String ifNamePattern : IF_NAME_PATTERNS) {
 +            commonPattern.append("|(").append(ifNamePattern).append(".*)");
 +        }
 +
 +        return fname.matches(commonPattern.toString());
 +    }
 +
++    protected boolean isBroadcastTypeVlanOrVxlan(final NicTO nic) {
++        return nic != null && (nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan
++                || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan);
++    }
++
++    protected boolean isValidProtocolAndVnetId(final String vNetId, final String protocol) {
++        return vNetId != null && protocol != null && !vNetId.equalsIgnoreCase("untagged");
++    }
++
 +    @Override
 +    public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicAdapter, Map<String, String> extraConfig) throws InternalErrorException, LibvirtException {
 +
 +        if (s_logger.isDebugEnabled()) {
 +            s_logger.debug("nic=" + nic);
 +            if (nicAdapter != null && !nicAdapter.isEmpty()) {
 +                s_logger.debug("custom nic adapter=" + nicAdapter);
 +            }
 +        }
 +
 +        LibvirtVMDef.InterfaceDef intf = new LibvirtVMDef.InterfaceDef();
 +
 +        String vNetId = null;
 +        String protocol = null;
-         if (nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan) {
++        if (isBroadcastTypeVlanOrVxlan(nic)) {
 +            vNetId = Networks.BroadcastDomainType.getValue(nic.getBroadcastUri());
 +            protocol = Networks.BroadcastDomainType.getSchemeValue(nic.getBroadcastUri()).scheme();
 +        } else if (nic.getBroadcastType() == Networks.BroadcastDomainType.Lswitch) {
 +            throw new InternalErrorException("Nicira NVP Logicalswitches are not supported by the BridgeVifDriver");
 +        }
 +        String trafficLabel = nic.getName();
 +        Integer networkRateKBps = 0;
 +        if (libvirtVersion > ((10 * 1000 + 10))) {
 +            networkRateKBps = (nic.getNetworkRateMbps() != null && nic.getNetworkRateMbps().intValue() != -1) ? nic.getNetworkRateMbps().intValue() * 128 : 0;
 +        }
 +
 +        if (nic.getType() == Networks.TrafficType.Guest) {
-             if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan) && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged")) ||
-                     (nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan)) {
++            if (isBroadcastTypeVlanOrVxlan(nic) && isValidProtocolAndVnetId(vNetId, protocol)) {
 +                    if (trafficLabel != null && !trafficLabel.isEmpty()) {
 +                        s_logger.debug("creating a vNet dev and bridge for guest traffic per traffic label " + trafficLabel);
 +                        String brName = createVnetBr(vNetId, trafficLabel, protocol);
 +                        intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter), networkRateKBps);
 +                    } else {
 +                        String brName = createVnetBr(vNetId, "private", protocol);
 +                        intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter), networkRateKBps);
 +                    }
 +            } else {
 +                String brname = "";
 +                if (trafficLabel != null && !trafficLabel.isEmpty()) {
 +                    brname = trafficLabel;
 +                } else {
 +                    brname = _bridges.get("guest");
 +                }
 +                intf.defBridgeNet(brname, null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter), networkRateKBps);
 +            }
 +        } else if (nic.getType() == Networks.TrafficType.Control) {
 +            /* Make sure the network is still there */
 +            createControlNetwork();
 +            intf.defBridgeNet(_bridges.get("linklocal"), null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter));
 +        } else if (nic.getType() == Networks.TrafficType.Public) {
-             if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan) && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged")) ||
-                     (nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan)) {
++            if (isBroadcastTypeVlanOrVxlan(nic) && isValidProtocolAndVnetId(vNetId, protocol)) {
 +                if (trafficLabel != null && !trafficLabel.isEmpty()) {
 +                    s_logger.debug("creating a vNet dev and bridge for public traffic per traffic label " + trafficLabel);
 +                    String brName = createVnetBr(vNetId, trafficLabel, protocol);
 +                    intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter), networkRateKBps);
 +                } else {
 +                    String brName = createVnetBr(vNetId, "public", protocol);
 +                    intf.defBridgeNet(brName, null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter), networkRateKBps);
 +                }
 +            } else {
 +                intf.defBridgeNet(_bridges.get("public"), null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter), networkRateKBps);
 +            }
 +        } else if (nic.getType() == Networks.TrafficType.Management) {
 +            intf.defBridgeNet(_bridges.get("private"), null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter));
 +        } else if (nic.getType() == Networks.TrafficType.Storage) {
 +            String storageBrName = nic.getName() == null ? _bridges.get("private") : nic.getName();
 +            intf.defBridgeNet(storageBrName, null, nic.getMac(), getGuestNicModel(guestOsType, nicAdapter));
 +        }
-         if (nic.getPxeDisable() == true) {
++        if (nic.getPxeDisable()) {
 +            intf.setPxeDisable(true);
 +        }
 +
 +        return intf;
 +    }
 +
 +    @Override
 +    public void unplug(LibvirtVMDef.InterfaceDef iface) {
 +        deleteVnetBr(iface.getBrName());
 +    }
 +
 +    @Override
 +    public void attach(LibvirtVMDef.InterfaceDef iface) {
 +        Script.runSimpleBashScript("brctl addif " + iface.getBrName() + " " + iface.getDevName());
 +    }
 +
 +    @Override
 +    public void detach(LibvirtVMDef.InterfaceDef iface) {
 +        Script.runSimpleBashScript("test -d /sys/class/net/" + iface.getBrName() + "/brif/" + iface.getDevName() + " && brctl delif " + iface.getBrName() + " " + iface.getDevName());
 +    }
 +
 +    private String generateVnetBrName(String pifName, String vnetId) {
 +        return "br" + pifName + "-" + vnetId;
 +    }
 +
 +    private String generateVxnetBrName(String pifName, String vnetId) {
 +        return "brvx-" + vnetId;
 +    }
 +
 +    private String createVnetBr(String vNetId, String pifKey, String protocol) throws InternalErrorException {
 +        String nic = _pifs.get(pifKey);
 +        if (nic == null) {
 +            // if not found in bridge map, maybe traffic label refers to pif already?
 +            File pif = new File("/sys/class/net/" + pifKey);
 +            if (pif.isDirectory()) {
 +                nic = pifKey;
 +            }
 +        }
 +        String brName = "";
 +        if (protocol.equals(Networks.BroadcastDomainType.Vxlan.scheme())) {
 +            brName = generateVxnetBrName(nic, vNetId);
 +        } else {
 +            brName = generateVnetBrName(nic, vNetId);
 +        }
 +        createVnet(vNetId, nic, brName, protocol);
 +        return brName;
 +    }
 +
 +    private void createVnet(String vnetId, String pif, String brName, String protocol) throws InternalErrorException {
 +        synchronized (_vnetBridgeMonitor) {
 +            String script = _modifyVlanPath;
 +            if (protocol.equals(Networks.BroadcastDomainType.Vxlan.scheme())) {
 +                script = _modifyVxlanPath;
 +            }
 +            final Script command = new Script(script, _timeout, s_logger);
 +            command.add("-v", vnetId);
 +            command.add("-p", pif);
 +            command.add("-b", brName);
 +            command.add("-o", "add");
 +
 +            final String result = command.execute();
 +            if (result != null) {
 +                throw new InternalErrorException("Failed to create vnet " + vnetId + ": " + result);
 +            }
 +        }
 +    }
 +
 +    private void deleteVnetBr(String brName) {
 +        synchronized (_vnetBridgeMonitor) {
 +            String cmdout = Script.runSimpleBashScript("ls /sys/class/net/" + brName);
 +            if (cmdout == null)
 +                // Bridge does not exist
 +                return;
 +            cmdout = Script.runSimpleBashScript("ls /sys/class/net/" + brName + "/brif | tr '\n' ' '");
 +            if (cmdout != null && cmdout.contains("vnet")) {
 +                // Active VM remains on that bridge
 +                return;
 +            }
 +
 +            Pattern oldStyleBrNameRegex = Pattern.compile("^cloudVirBr(\\d+)$");
 +            Pattern brNameRegex = Pattern.compile("^br(\\S+)-(\\d+)$");
 +            Matcher oldStyleBrNameMatcher = oldStyleBrNameRegex.matcher(brName);
 +            Matcher brNameMatcher = brNameRegex.matcher(brName);
 +
 +            String pName = null;
 +            String vNetId = null;
 +            if (oldStyleBrNameMatcher.find()) {
 +                // Actually modifyvlan.sh doesn't require pif name when deleting its bridge so far.
 +                pName = "undefined";
 +                vNetId = oldStyleBrNameMatcher.group(1);
 +            } else if (brNameMatcher.find()) {
 +                if (brNameMatcher.group(1) != null || !brNameMatcher.group(1).isEmpty()) {
 +                    pName = brNameMatcher.group(1);
 +                } else {
 +                    pName = "undefined";
 +                }
 +                vNetId = brNameMatcher.group(2);
 +            }
 +
 +            if (vNetId == null || vNetId.isEmpty()) {
 +                s_logger.debug("unable to get a vNet ID from name " + brName);
 +                return;
 +            }
 +
 +            String scriptPath = null;
 +            if (cmdout != null && cmdout.contains("vxlan")) {
 +                scriptPath = _modifyVxlanPath;
 +            } else {
 +                scriptPath = _modifyVlanPath;
 +            }
 +
 +            final Script command = new Script(scriptPath, _timeout, s_logger);
 +            command.add("-o", "delete");
 +            command.add("-v", vNetId);
 +            command.add("-p", pName);
 +            command.add("-b", brName);
 +
 +            final String result = command.execute();
 +            if (result != null) {
 +                s_logger.debug("Delete bridge " + brName + " failed: " + result);
 +            }
 +        }
 +    }
 +
 +    private void deleteExistingLinkLocalRouteTable(String linkLocalBr) {
 +        Script command = new Script("/bin/bash", _timeout);
 +        command.add("-c");
 +        command.add("ip route | grep " + NetUtils.getLinkLocalCIDR());
 +        OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
 +        String result = command.execute(parser);
 +        boolean foundLinkLocalBr = false;
 +        if (result == null && parser.getLines() != null) {
 +            String[] lines = parser.getLines().split("\\n");
 +            for (String line : lines) {
 +                String[] tokens = line.split(" ");
 +                if (tokens != null && tokens.length < 2) {
 +                    continue;
 +                }
 +                final String device = tokens[2];
 +                if (!Strings.isNullOrEmpty(device) && !device.equalsIgnoreCase(linkLocalBr)) {
 +                    Script.runSimpleBashScript("ip route del " + NetUtils.getLinkLocalCIDR() + " dev " + tokens[2]);
 +                } else {
 +                    foundLinkLocalBr = true;
 +                }
 +            }
 +        }
 +        if (!foundLinkLocalBr) {
 +            Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " +
 +                NetUtils.getLinkLocalGateway());
 +        }
 +    }
 +
 +    private void createControlNetwork() {
 +        createControlNetwork(_bridges.get("linklocal"));
 +    }
 +
 +    @Override
 +    public void createControlNetwork(String privBrName)  {
 +        deleteExistingLinkLocalRouteTable(privBrName);
 +        if (!isExistingBridge(privBrName)) {
 +            Script.runSimpleBashScript("brctl addbr " + privBrName + "; ip link set " + privBrName + " up; ip address add 169.254.0.1/16 dev " + privBrName, _timeout);
 +        }
 +    }
 +
 +    @Override
 +    public boolean isExistingBridge(String bridgeName) {
 +        File f = new File("/sys/devices/virtual/net/" + bridgeName + "/bridge");
 +        if (f.exists()) {
 +            return true;
 +        } else {
 +            return false;
 +        }
 +    }
 +}
diff --cc plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java
index 0000000,ad0f92b..ad0f92b
mode 000000,100644..100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java
diff --cc scripts/vm/network/security_group.py
index 0471dbe,e9f49d4..12afe0b
--- a/scripts/vm/network/security_group.py
+++ b/scripts/vm/network/security_group.py
@@@ -405,13 -435,19 +420,20 @@@ def network_rules_vmSecondaryIp(vm_name
      domid = getvmId(vm_name)
  
      vmchain = vm_name
-     add_to_ipset(vmchain, [ip_secondary], action)
+     vmchain6 = vmchain + '-6'
+ 
+     ip4s, ip6s = split_ips_by_family(ip_secondary)
+ 
+     add_to_ipset(vmchain, ip4s, action)
+ 
+     #add ebtables rules for the secondary ips
+     ebtables_rules_vmip(vm_name, ip4s, action)
  
-     #add ebtables rules for the secondary ip
-     ebtables_rules_vmip(vm_name, [ip_secondary], action)
+     #add ipv6 addresses to ipv6 ipset
+     add_to_ipset(vmchain6, ip6s, action)
  
 -    return 'true'
 +    return True
 +
  
  def ebtables_rules_vmip (vmname, ips, action):
      eb_vm_chain=ebtables_chain_name(vmname)
@@@ -460,27 -495,34 +482,35 @@@ def default_network_rules(vm_name, vm_i
  
      action = "-A"
      vmipsetName = ipset_chain_name(vm_name)
+     vmipsetName6 = vmipsetName + '-6'
+ 
      #create ipset and add vm ips to that ip set
 -    if create_ipset_forvm(vmipsetName) == False:
 +    if not create_ipset_forvm(vmipsetName):
         logging.debug(" failed to create ipset for rule " + str(tokens))
 -       return 'false'
 +       return False
  
      #add primary nic ip to ipset
 -    if add_to_ipset(vmipsetName, [vm_ip], action ) == False:
 +    if not add_to_ipset(vmipsetName, [vm_ip], action ):
         logging.debug(" failed to add vm " + vm_ip + " ip to set ")
 -       return 'false'
 +       return False
  
      #add secodnary nic ips to ipset
      secIpSet = "1"
      ips = sec_ips.split(';')
      ips.pop()
-     if ips[0] == "0":
++
+     if len(ips) == 0 or ips[0] == "0":
          secIpSet = "0"
+         ip4s = []
+         ip6s = []
  
      if secIpSet == "1":
-         logging.debug("Adding ipset for secondary ips")
-         add_to_ipset(vmipsetName, ips, action)
-         if not write_secip_log_for_vm(vm_name, sec_ips, vm_id):
+         logging.debug("Adding ipset for secondary ipv4 addresses")
+         ip4s, ip6s = split_ips_by_family(ips)
+ 
+         add_to_ipset(vmipsetName, ip4s, action)
+ 
+         if write_secip_log_for_vm(vm_name, sec_ips, vm_id) == False:
              logging.debug("Failed to log default network rules, ignoring")
  
      try:
@@@ -505,17 -547,15 +535,15 @@@
  
      default_ebtables_rules(vm_name, vm_ip, vm_mac, vif)
      #default ebtables rules for vm secondary ips
-     ebtables_rules_vmip(vm_name, ips, "-I")
+     ebtables_rules_vmip(vm_name, ip4s, "-I")
  
 -    if vm_ip is not None:
 -        if write_rule_log_for_vm(vmName, vm_id, vm_ip, domID, '_initial_', '-1') == False:
 +    if vm_ip:
 +        if not write_rule_log_for_vm(vmName, vm_id, vm_ip, domID, '_initial_', '-1'):
              logging.debug("Failed to log default network rules, ignoring")
  
-     vm_ip6_set_name = vm_name + '-6'
- 
-     if not create_ipset_forvm(vm_ip6_set_name, family='inet6', type='hash:net'):
+     if not create_ipset_forvm(vmipsetName6, family='inet6', type='hash:net'):
         logging.debug(" failed to create ivp6 ipset for rule " + str(tokens))
 -       return 'false'
 +       return False
  
      vm_ip6_addr = [ipv6_link_local]
      try: