From a84f7dfde9d06e858db3151ca6fc10b1aaf231b0 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 28 Nov 2018 00:17:13 +0530 Subject: [PATCH 1/3] marvin: add missing default test data (#3055) This add missing test data for one of the keys for a recently added migration test. Signed-off-by: Rohit Yadav --- tools/marvin/marvin/config/test_data.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/marvin/marvin/config/test_data.py b/tools/marvin/marvin/config/test_data.py index 11595e4a26f..b8ebcb6cbaf 100644 --- a/tools/marvin/marvin/config/test_data.py +++ b/tools/marvin/marvin/config/test_data.py @@ -473,6 +473,12 @@ test_data = { "supportedservices": "Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,NetworkACL" }, + "vpc_offering_reduced": { + "name": "VPC reduced off", + "displaytext": "VPC reduced off", + "supportedservices": + "Dhcp,Dns,SourceNat,UserData,StaticNat,NetworkACL" + }, "vpc_offering_multi_lb": { "name": "VPC offering with multiple Lb service providers", "displaytext": "VPC offering with multiple Lb service providers", From d425a409fc070468d73aa3b7fe5ea126823dec1a Mon Sep 17 00:00:00 2001 From: Rene Diepstraten Date: Wed, 28 Nov 2018 15:00:13 +0100 Subject: [PATCH 2/3] sg: add secondary ips to the correct ipset based on ip family (#2990) Currently secondary ipv6 addresses are added to the ipv4 ipset in security_group.py. This doesn't work, so this patch adds a function to split a set of ips in ipv4 and ipv6 addresses. Both the default_network_rules and network_rules_vmSecondaryIp functions now utilise this function and add the ips to the appropriate ipsets. --- scripts/vm/network/security_group.py | 72 ++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/scripts/vm/network/security_group.py b/scripts/vm/network/security_group.py index 19aa7359081..e9f49d4a309 100755 --- a/scripts/vm/network/security_group.py +++ b/scripts/vm/network/security_group.py @@ -189,6 +189,21 @@ def ipv6_link_local_addr(mac=None): return IPAddress('fe80::' + ':'.join(re.findall(r'.{4}', eui64))) +def split_ips_by_family(ips): + if type(ips) is str: + ips = [ip for ip in ips.split(';') if ip != ''] + + ip4s = [] + ip6s = [] + for ip in ips: + version = IPNetwork(ip).version + if version == 4: + ip4s.append(ip) + elif version == 6: + ip6s.append(ip) + return ip4s, ip6s + + def destroy_network_rules_for_vm(vm_name, vif=None): vmchain = iptables_chain_name(vm_name) vmchain_egress = egress_chain_name(vm_name) @@ -420,10 +435,17 @@ def network_rules_vmSecondaryIp(vm_name, ip_secondary, action): domid = getvmId(vm_name) vmchain = vm_name - add_to_ipset(vmchain, [ip_secondary], action) + vmchain6 = vmchain + '-6' - #add ebtables rules for the secondary ip - ebtables_rules_vmip(vm_name, [ip_secondary], action) + 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 ipv6 addresses to ipv6 ipset + add_to_ipset(vmchain6, ip6s, action) return 'true' @@ -473,6 +495,8 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, se 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: logging.debug(" failed to create ipset for rule " + str(tokens)) @@ -487,12 +511,17 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, se secIpSet = "1" ips = sec_ips.split(';') ips.pop() - if ips[0] == "0": - secIpSet = "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) + 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") @@ -518,15 +547,13 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, se 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: 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' @@ -538,7 +565,10 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, se except AddrFormatError: pass - add_to_ipset(vm_ip6_set_name, vm_ip6_addr, action) + add_to_ipset(vmipsetName6, vm_ip6_addr, action) + if secIpSet == "1": + logging.debug("Adding ipset for secondary ipv6 addresses") + add_to_ipset(vmipsetName6, ip6s, action) try: execute('ip6tables -A ' + brfw + '-OUT' + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -j ' + vmchain_default) @@ -553,20 +583,20 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, se # Allow neighbor solicitations and advertisements execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type neighbor-solicitation -m hl --hl-eq 255 -j RETURN') execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -p icmpv6 --icmpv6-type neighbor-solicitation -m hl --hl-eq 255 -j ACCEPT') - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type neighbor-advertisement -m set --match-set ' + vm_ip6_set_name + ' src -m hl --hl-eq 255 -j RETURN') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type neighbor-advertisement -m set --match-set ' + vmipsetName6 + ' src -m hl --hl-eq 255 -j RETURN') execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -p icmpv6 --icmpv6-type neighbor-advertisement -m hl --hl-eq 255 -j ACCEPT') # Packets to allow as per RFC4890 - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type packet-too-big -m set --match-set ' + vm_ip6_set_name + ' src -j RETURN') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type packet-too-big -m set --match-set ' + vmipsetName6 + ' src -j RETURN') execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -p icmpv6 --icmpv6-type packet-too-big -j ACCEPT') - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type destination-unreachable -m set --match-set ' + vm_ip6_set_name + ' src -j RETURN') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type destination-unreachable -m set --match-set ' + vmipsetName6 + ' src -j RETURN') execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -p icmpv6 --icmpv6-type destination-unreachable -j ACCEPT') - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type time-exceeded -m set --match-set ' + vm_ip6_set_name + ' src -j RETURN') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type time-exceeded -m set --match-set ' + vmipsetName6 + ' src -j RETURN') execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -p icmpv6 --icmpv6-type time-exceeded -j ACCEPT') - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type parameter-problem -m set --match-set ' + vm_ip6_set_name + ' src -j RETURN') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p icmpv6 --icmpv6-type parameter-problem -m set --match-set ' + vmipsetName6 + ' src -j RETURN') execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -p icmpv6 --icmpv6-type parameter-problem -j ACCEPT') # MLDv2 discovery packets @@ -578,14 +608,14 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, se execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p udp --sport 547 ! --dst fe80::/64 -j DROP') # Always allow outbound DNS over UDP and TCP - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p udp --dport 53 -m set --match-set ' + vm_ip6_set_name + ' src -j RETURN') - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p tcp --dport 53 -m set --match-set ' + vm_ip6_set_name + ' src -j RETURN') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p udp --dport 53 -m set --match-set ' + vmipsetName6 + ' src -j RETURN') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -p tcp --dport 53 -m set --match-set ' + vmipsetName6 + ' src -j RETURN') # Prevent source address spoofing - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -m set ! --match-set ' + vm_ip6_set_name + ' src -j DROP') + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -m set ! --match-set ' + vmipsetName6 + ' src -j DROP') # Send proper traffic to the egress chain of the Instance - execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -m set --match-set ' + vm_ip6_set_name + ' src -j ' + vmchain_egress) + execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-in ' + vif + ' -m set --match-set ' + vmipsetName6 + ' src -j ' + vmchain_egress) execute('ip6tables -A ' + vmchain_default + ' -m physdev --physdev-is-bridged --physdev-out ' + vif + ' -j ' + vmchain) From 29b8a9da48ee4abf73f5d28342b246e9983618a9 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 28 Nov 2018 22:22:30 +0530 Subject: [PATCH 3/3] kvm: when untagged vxlan is used, use the default guest/public bridge (#3037) When vxlan://untagged is used for public (or guest) network, use the default public/guest bridge device same as how vlan://untagged works. Signed-off-by: Rohit Yadav --- .../kvm/resource/BridgeVifDriver.java | 19 ++++-- .../kvm/resource/BridgeVifDriverTest.java | 58 +++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java 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 11b22c494f4..bd410df419e 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 @@ -206,6 +206,15 @@ public class BridgeVifDriver extends VifDriverBase { 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) throws InternalErrorException, LibvirtException { @@ -220,7 +229,7 @@ public class BridgeVifDriver extends VifDriverBase { 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) { @@ -233,8 +242,7 @@ public class BridgeVifDriver extends VifDriverBase { } 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); @@ -257,8 +265,7 @@ public class BridgeVifDriver extends VifDriverBase { 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); @@ -276,7 +283,7 @@ public class BridgeVifDriver extends VifDriverBase { 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); } diff --git a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java new file mode 100644 index 00000000000..ad0f92bd3f8 --- /dev/null +++ b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/BridgeVifDriverTest.java @@ -0,0 +1,58 @@ +// 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 org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.cloud.agent.api.to.NicTO; +import com.cloud.network.Networks; + +public class BridgeVifDriverTest { + + private BridgeVifDriver driver; + + @Before + public void setUp() throws Exception { + driver = new BridgeVifDriver(); + } + + @Test + public void isBroadcastTypeVlanOrVxlan() { + final NicTO nic = new NicTO(); + nic.setBroadcastType(Networks.BroadcastDomainType.Native); + Assert.assertFalse(driver.isBroadcastTypeVlanOrVxlan(null)); + Assert.assertFalse(driver.isBroadcastTypeVlanOrVxlan(nic)); + // Test VLAN + nic.setBroadcastType(Networks.BroadcastDomainType.Vlan); + Assert.assertTrue(driver.isBroadcastTypeVlanOrVxlan(nic)); + // Test VXLAN + nic.setBroadcastType(Networks.BroadcastDomainType.Vxlan); + Assert.assertTrue(driver.isBroadcastTypeVlanOrVxlan(nic)); + } + + @Test + public void isValidProtocolAndVnetId() { + Assert.assertFalse(driver.isValidProtocolAndVnetId(null, null)); + Assert.assertFalse(driver.isValidProtocolAndVnetId("123", null)); + Assert.assertFalse(driver.isValidProtocolAndVnetId(null, "vlan")); + Assert.assertFalse(driver.isValidProtocolAndVnetId("untagged", "vxlan")); + Assert.assertTrue(driver.isValidProtocolAndVnetId("123", "vlan")); + Assert.assertTrue(driver.isValidProtocolAndVnetId("456", "vxlan")); + } +} \ No newline at end of file