[ https://issues.apache.org/jira/browse/CLOUDSTACK-10127?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16301200#comment-16301200 ]
ASF GitHub Bot commented on CLOUDSTACK-10127: --------------------------------------------- rhtyd closed pull request #2304: CLOUDSTACK-10127: KVM + Ovs: Incorrect devId on static nat URL: https://github.com/apache/cloudstack/pull/2304 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): 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 1db9d67afc2..db47f466bfe 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -1596,27 +1596,20 @@ protected ExecutionResult prepareNetworkElementCommand(final IpAssocVpcCommand c final String routerName = cmd.getAccessDetail(NetworkElementCommand.ROUTER_NAME); try { - conn = LibvirtConnection.getConnectionByVmName(routerName); + conn = getLibvirtUtilitiesHelper().getConnectionByVmName(routerName); final IpAddressTO[] ips = cmd.getIpAddresses(); Integer devNum = 0; - final Map<String, Integer> broadcastUriToNicNum = new HashMap<String, Integer>(); final List<InterfaceDef> pluggedNics = getInterfaces(conn, routerName); + final Map<String, Integer> macAddressToNicNum = new HashMap<>(pluggedNics.size()); for (final InterfaceDef pluggedNic : pluggedNics) { final String pluggedVlan = pluggedNic.getBrName(); - if (pluggedVlan.equalsIgnoreCase(_linkLocalBridgeName)) { - broadcastUriToNicNum.put("LinkLocal", devNum); - } else if (pluggedVlan.equalsIgnoreCase(_publicBridgeName) || pluggedVlan.equalsIgnoreCase(_privBridgeName) || - pluggedVlan.equalsIgnoreCase(_guestBridgeName)) { - broadcastUriToNicNum.put(BroadcastDomainType.Vlan.toUri(Vlan.UNTAGGED).toString(), devNum); - } else { - broadcastUriToNicNum.put(getBroadcastUriFromBridge(pluggedVlan), devNum); - } + macAddressToNicNum.put(pluggedNic.getMacAddress(), devNum); devNum++; } for (final IpAddressTO ip : ips) { - ip.setNicDevId(broadcastUriToNicNum.get(ip.getBroadcastUri())); + ip.setNicDevId(macAddressToNicNum.get(ip.getVifMacAddress())); } return new ExecutionResult(true, null); diff --git a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkElementCommandWrapperTest.java b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkElementCommandWrapperTest.java new file mode 100644 index 00000000000..4d068350f3f --- /dev/null +++ b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkElementCommandWrapperTest.java @@ -0,0 +1,280 @@ +// +// 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.wrapper; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import javax.naming.ConfigurationException; + +import org.junit.Before; +import org.junit.Test; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.LibvirtException; + +import com.cloud.agent.api.routing.IpAssocVpcCommand; +import com.cloud.agent.api.routing.NetworkElementCommand; +import com.cloud.agent.api.to.IpAddressTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.network.Networks; +import com.cloud.utils.ExecutionResult; + +public class LibvirtNetworkElementCommandWrapperTest { + private static final String fullfile = "<domain type='kvm' id='143'>\n" + + " <name>r-3-VM</name>\n" + + " <uuid>8825b180-468f-4227-beb7-6b06fd342116</uuid>\n" + + " <description>CentOS 5.5 (64-bit)</description>\n" + + " <memory unit='KiB'>262144</memory>\n" + + " <currentMemory unit='KiB'>262144</currentMemory>\n" + + " <vcpu placement='static'>1</vcpu>\n" + + " <cputune>\n" + + " <shares>256</shares>\n" + + " </cputune>\n" + + " <sysinfo type='smbios'>\n" + + " <system>\n" + + " <entry name='manufacturer'>Apache Software Foundation</entry>\n" + + " <entry name='product'>CloudStack KVM Hypervisor</entry>\n" + + " <entry name='uuid'>8825b180-468f-4227-beb7-6b06fd342116</entry>\n" + + " </system>\n" + + " </sysinfo>\n" + + " <os>\n" + + " <type arch='x86_64' machine='pc-i440fx-rhel7.0.0'>hvm</type>\n" + + " <boot dev='cdrom'/>\n" + + " <boot dev='hd'/>\n" + + " <smbios mode='sysinfo'/>\n" + + " </os>\n" + + " <features>\n" + + " <acpi/>\n" + + " <apic/>\n" + + " <pae/>\n" + + " </features>\n" + + " <clock offset='utc'>\n" + + " <timer name='kvmclock'/>\n" + + " </clock>\n" + + " <on_poweroff>destroy</on_poweroff>\n" + + " <on_reboot>restart</on_reboot>\n" + + " <on_crash>destroy</on_crash>\n" + + " <devices>\n" + + " <emulator>/usr/libexec/qemu-kvm</emulator>\n" + + " <disk type='file' device='disk'>\n" + + " <driver name='qemu' type='qcow2' cache='none'/>\n" + + " <source file='/mnt/4436eeec-abec-3ef8-b733-c9541df20361/0c4aae69-2652-4a04-b460-1abb5a1a695c'/>\n" + + " <backingStore type='file' index='1'>\n" + + " <format type='raw'/>\n" + + " <source file='/mnt/4436eeec-abec-3ef8-b733-c9541df20361/d9ce07e5-9e13-11e7-816b-faac09070700'/>\n" + + " <backingStore/>\n" + + " </backingStore>\n" + + " <target dev='vda' bus='virtio'/>\n" + + " <serial>0c4aae6926524a04b460</serial>\n" + + " <alias name='virtio-disk0'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>\n" + + " </disk>\n" + + " <disk type='file' device='cdrom'>\n" + + " <driver name='qemu' type='raw' cache='none'/>\n" + + " <backingStore/>\n" + + " <target dev='hdc' bus='ide'/>\n" + + " <readonly/>\n" + + " <alias name='ide0-1-0'/>\n" + + " <address type='drive' controller='0' bus='1' target='0' unit='0'/>\n" + + " </disk>\n" + + " <controller type='usb' index='0'>\n" + + " <alias name='usb'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>\n" + + " </controller>\n" + + " <controller type='pci' index='0' model='pci-root'>\n" + + " <alias name='pci.0'/>\n" + + " </controller>\n" + + " <controller type='ide' index='0'>\n" + + " <alias name='ide'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>\n" + + " </controller>\n" + + " <controller type='virtio-serial' index='0'>\n" + + " <alias name='virtio-serial0'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>\n" + + " </controller>\n" + + " <interface type='bridge'>\n" + + " <mac address='02:00:7c:98:00:01'/>\n" + + " <source bridge='cloud0'/>\n" + + " <bandwidth>\n" + + " <inbound average='25600' peak='25600'/>\n" + + " <outbound average='25600' peak='25600'/>\n" + + " </bandwidth>\n" + + " <target dev='vnet1'/>\n" + + " <model type='virtio'/>\n" + + " <virtualport type='openvswitch'>\n" + + " </virtualport>\n" + + " <link state='up'/>\n" + + " <alias name='net0'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>\n" + + " </interface>\n" + + " <interface type='bridge'>\n" + + " <mac address='02:00:7c:98:00:02'/>\n" + + " <source bridge='publicbr'/>\n" + + " <bandwidth>\n" + + " <inbound average='25600' peak='25600'/>\n" + + " <outbound average='25600' peak='25600'/>\n" + + " </bandwidth>\n" + + " <target dev='vnet2'/>\n" + + " <model type='virtio'/>\n" + + " <virtualport type='openvswitch'>\n" + + " </virtualport>\n" + + " <link state='up'/>\n" + + " <alias name='net1'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>\n" + + " </interface>\n" + + " <interface type='bridge'>\n" + + " <mac address='02:00:7c:98:00:03'/>\n" + + " <source bridge='guestbr-100'/>\n" + + " <bandwidth>\n" + + " <inbound average='25600' peak='25600'/>\n" + + " <outbound average='25600' peak='25600'/>\n" + + " </bandwidth>\n" + + " <target dev='vnet3'/>\n" + + " <model type='virtio'/>\n" + + " <virtualport type='openvswitch'>\n" + + " </virtualport>\n" + + " <link state='up'/>\n" + + " <alias name='net2'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>\n" + + " </interface>\n" + + " <interface type='bridge'>\n" + + " <mac address='02:00:7c:98:00:04'/>\n" + + " <source bridge='guestbr-101'/>\n" + + " <bandwidth>\n" + + " <inbound average='25600' peak='25600'/>\n" + + " <outbound average='25600' peak='25600'/>\n" + + " </bandwidth>\n" + + " <target dev='vnet4'/>\n" + + " <model type='virtio'/>\n" + + " <virtualport type='openvswitch'>\n" + + " </virtualport>\n" + + " <link state='up'/>\n" + + " <alias name='net3'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>\n" + + " </interface>\n" + + " <serial type='pty'>\n" + + " <source path='/dev/pts/4'/>\n" + + " <target port='0'/>\n" + + " <alias name='serial0'/>\n" + + " </serial>\n" + + " <console type='pty' tty='/dev/pts/4'>\n" + + " <source path='/dev/pts/4'/>\n" + + " <target type='serial' port='0'/>\n" + + " <alias name='serial0'/>\n" + + " </console>\n" + + " <channel type='unix'>\n" + + " <source mode='bind' path='/var/lib/libvirt/qemu/i-85-285-VM.org.qemu.guest_agent.0'/>\n" + + " <target type='virtio' name='org.qemu.guest_agent.0' state='disconnected'/>\n" + + " <alias name='channel0'/>\n" + + " <address type='virtio-serial' controller='0' bus='0' port='1'/>\n" + + " </channel>\n" + + " <input type='tablet' bus='usb'>\n" + + " <alias name='input0'/>\n" + + " </input>\n" + + " <input type='mouse' bus='ps2'/>\n" + + " <input type='keyboard' bus='ps2'/>\n" + + " <graphics type='vnc' port='5903' autoport='yes' listen='10.100.100.11'>\n" + + " <listen type='address' address='10.100.100.11'/>\n" + + " </graphics>\n" + + " <video>\n" + + " <model type='cirrus' vram='16384' heads='1'/>\n" + + " <alias name='video0'/>\n" + + " <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>\n" + + " </video>\n" + + " <memballoon model='none'>\n" + + " <alias name='balloon0'/>\n" + + " </memballoon>\n" + + " </devices>\n" + + "</domain>\n"; + + private LibvirtComputingResource res; + private final Domain _domain = mock(Domain.class); + + @Before + public void setUp() throws LibvirtException, ConfigurationException { + // Use a spy because we only want to override getVifDriverClass + LibvirtComputingResource resReal = new LibvirtComputingResource() { + { + _linkLocalBridgeName = "cloud0"; + _guestBridgeName = "guestbr"; + _publicBridgeName = "publicbr"; + _privBridgeName = "mgmtbr"; + } + }; + + res = spy(resReal); + + Connect conn = mock(Connect.class); + LibvirtUtilitiesHelper helper = mock(LibvirtUtilitiesHelper.class); + + when(_domain.getXMLDesc(0)).thenReturn(fullfile); + when(conn.domainLookupByName(anyString())).thenReturn(_domain); + when(helper.getConnectionByVmName(anyString())).thenReturn(conn); + + doReturn(helper).when(res).getLibvirtUtilitiesHelper(); + } + + @Test + public void testPrepareIpAssocVpcCommand() throws LibvirtException { + IpAddressTO ip = new IpAddressTO(1, "171.31.1.3", + true, false, false, + "vlan://untagged", + "172.31.1.1", + "255.255.0.0", + "02:00:7c:98:00:02", + 0, + true); + ip.setTrafficType(Networks.TrafficType.Public); + IpAddressTO[] ips = new IpAddressTO[] { + ip + }; + final IpAssocVpcCommand command = new IpAssocVpcCommand(ips); + command.setAccessDetail(NetworkElementCommand.ROUTER_IP, "127.0.0.1"); + ExecutionResult result = res.prepareCommand(command); + + assertEquals(1, ips[0].getNicDevId().intValue()); + } + + @Test + public void testVpcPrivateGateway() throws LibvirtException { + IpAddressTO ip = new IpAddressTO(1, "171.31.1.3", + true, false, false, + "vlan://untagged", + "172.31.1.1", + "255.255.0.0", + "02:00:7c:98:00:03", + 0, + false); + ip.setTrafficType(Networks.TrafficType.Guest); + IpAddressTO[] ips = new IpAddressTO[] { + ip + }; + final IpAssocVpcCommand command = new IpAssocVpcCommand(ips); + command.setAccessDetail(NetworkElementCommand.ROUTER_IP, "127.0.0.1"); + ExecutionResult result = res.prepareCommand(command); + + assertEquals(2, ips[0].getNicDevId().intValue()); + } + +} ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > 4.9 / 4.10 KVM + openvswitch + vpc + static nat / secondary ip on eth2? > ------------------------------------------------------------------------ > > Key: CLOUDSTACK-10127 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10127 > Project: CloudStack > Issue Type: Bug > Security Level: Public(Anyone can view this level - this is the > default.) > Components: VPC > Affects Versions: 4.7.0, 4.8.0, 4.9.0, 4.10.0.0 > Environment: CentOS 7.4.1708 + KVM + OpenvSwitch 2.3-2.8 > Reporter: Sven Vogel > Assignee: Frank Maximus > Priority: Critical > Fix For: 4.11.0.0 > > > We have the following Problem. > 1. KVM > 2. Bridges > bond with two interfaces and trunk (0,129,180,100-1500) to cloudbr0 > Cloudbr0 (0 - guest network) > Fakebridge pub129 (public network) > Fakebridge sto180 (secondary storage network) > Fakebridge mgmt0 (management) > If I have a vpc all things work until I add a secondary ip and add a > static nat. > The following will happen, first address will be on the the correct > interface 146.0.122.134/26 but static nat will be on the false network. > Its on the eth2… > {{ root@r-29-VM:~# ip a > 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue state UNKNOWN > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > inet 127.0.0.1/8 scope host lo > 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state > UP qlen 1000 > link/ether 0e:00:a9:fe:03:81 brd ff:ff:ff:ff:ff:ff > inet 169.254.3.129/16 brd 169.254.255.255 scope global eth0 > 3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state > UP qlen 1000 > link/ether 1e:00:2c:00:00:68 brd ff:ff:ff:ff:ff:ff > inet 146.0.122.134/26 brd 146.0.122.191 scope global eth1 > 4: eth2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state > UP qlen 1000 > link/ether 02:00:57:07:00:0c brd ff:ff:ff:ff:ff:ff > inet 192.168.1.254/24 brd 192.168.1.255 scope global eth2 > inet 146.0.122.135/26 brd 146.0.122.191 scope global eth2}} > Normally I think the secondary ip should be on signed to eth1 not eth2! > It sets my ip on the guest network vlan range on my cloudbr0 but it should be > pub129. vnet6 has 1353 guest tag and not the public tag. > [root@kvm01 ~]# ovs-vsctl list-br > cloud0 > cloudbr0 > mgmt0 > pub129 > sto180 > [root@kvm01 ~]# virsh domiflist r-29-VM > Interface Type Source Model MAC > ------------------------------------------------------- > vnet4 bridge cloud0 virtio 0e:00:a9:fe:03:81 > vnet5 bridge pub129 virtio 1e:00:2c:00:00:68 > vnet6 bridge cloudbr0 virtio 02:00:57:07:00:0c > Bridge "cloud0" > Port "vnet4" > Interface "vnet4" > Port "vnet5" > tag: 129 > Interface "vnet5" > Port "vnet6" > tag: 1353 > Interface "vnet6" > root@r-29-VM:~# cat /etc/cloudstack/ips.json { > "eth0": [ > { > "add": true, > "broadcast": "169.254.255.255", > "cidr": "169.254.3.129/16", > "device": "eth0", > "gateway": "None", > "netmask": "255.255.0.0", > "network": "169.254.0.0/16", > "nic_dev_id": "0", > "nw_type": "control", > "one_to_one_nat": false, > "public_ip": "169.254.3.129", > "size": "16", > "source_nat": false > } > ], > "eth1": [ > { > "add": true, > "broadcast": "146.0.122.191", > "cidr": "146.0.122.134/26", > "device": "eth1", > "first_i_p": true, > "gateway": "146.0.122.130", > "netmask": "255.255.255.192", > "network": "146.0.122.128/26", > "new_nic": false, > "nic_dev_id": 1, > "nw_type": "public", > "one_to_one_nat": false, > "public_ip": "146.0.122.134", > "size": "26", > "source_nat": true, > "vif_mac_address": "1e:00:2c:00:00:68" > } > ], > "eth2": [ > { > "add": true, > "broadcast": "146.0.122.191", > "cidr": "146.0.122.135/26", > "device": "eth2", > "first_i_p": true, > "gateway": "146.0.122.130", > "netmask": "255.255.255.192", > "network": "146.0.122.128/26", > "new_nic": false, > "nic_dev_id": 2, > "nw_type": "public", > "one_to_one_nat": true, > "public_ip": "146.0.122.135", > "size": "26", > "source_nat": true, > "vif_mac_address": "1e:00:2c:00:00:68" > }, > { > "add": false, > "broadcast": "146.0.122.191", > "cidr": "146.0.122.136/26", > "device": "eth2", > "first_i_p": true, > "gateway": "146.0.122.130", > "netmask": "255.255.255.192", > "network": "146.0.122.128/26", > "new_nic": false, > "nic_dev_id": 2, > "nw_type": "public", > "one_to_one_nat": true, > "public_ip": "146.0.122.136", > "size": "26", > "source_nat": true, > "vif_mac_address": "1e:00:2c:00:00:68" > }, > { > "add": true, > "broadcast": "192.168.1.255", > "cidr": "192.168.1.254/24", > "device": "eth2", > "gateway": "192.168.1.254", > "netmask": "255.255.255.0", > "network": "192.168.1.0/24", > "nic_dev_id": "2", > "nw_type": "guest", > "one_to_one_nat": false, > "public_ip": "192.168.1.254", > "size": "24", > "source_nat": false > } > ], > "id": "ips" > } > Frank Maximus from Nuage analysed the problem. > {quote} > That seems to be a bug in the lookup of the device number, in case of > openvswitch. > The config clearly sets device to eth2, while it should be eth1. > More specifically: > in LibvirtComputingResource.prepareNetworkElementCommand() > The broadcastUriToNicNum map is filled depending on the VR nics. > In openvswitch the guest bridge is used as is, so it overwrites the mapping > of public. > This was not an issue until 4.6 as then VR was using the macaddress to do > lookup, while now it is using the device number. > Kind Regards, > Frank{quote} > I hope anyone can fix that fastly. -- This message was sent by Atlassian JIRA (v6.4.14#64029)