From 5c09abacce363baa7ce24a9432adf0ac5e9b2e70 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Sun, 18 Nov 2018 17:50:10 +0530 Subject: [PATCH 1/3] kvm: when untagged vxlan is used, use the default guest/public bridge 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 --- .../cloud/hypervisor/kvm/resource/BridgeVifDriver.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 11b22c494f46..340041cf6422 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 @@ -233,8 +233,8 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA } 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 ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan) + && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged"))) { 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 +257,8 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA 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 ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan) + && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged"))) { 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); From 26c9206be9761738dcaf1eef6ca8f0d6ff4dd07c Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Sat, 24 Nov 2018 15:26:05 +0530 Subject: [PATCH 2/3] kvm: refactor and add unit test Signed-off-by: Rohit Yadav --- .../kvm/resource/BridgeVifDriver.java | 17 ++++-- .../kvm/resource/BridgeVifDriverTest.java | 58 +++++++++++++++++++ 2 files changed, 70 insertions(+), 5 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 340041cf6422..1c5d1693618f 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 @@ private static boolean isInterface(final String fname) { 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 { @@ -233,8 +242,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA } if (nic.getType() == Networks.TrafficType.Guest) { - if ((nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan) - && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged"))) { + 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 LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA 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 || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan) - && (vNetId != null) && (protocol != null) && (!vNetId.equalsIgnoreCase("untagged"))) { + 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 LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA 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 000000000000..ad0f92bd3f88 --- /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 From e584c57d0ade3bde49ef8cc7d83a950b73d28b6c Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 28 Nov 2018 00:42:38 +0530 Subject: [PATCH 3/3] Update BridgeVifDriver.java --- .../src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1c5d1693618f..bd410df419e3 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 @@ -229,7 +229,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA 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) {