You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Marcus Sorensen <sh...@gmail.com> on 2013/10/29 07:58:48 UTC

Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/
-----------------------------------------------------------

Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.


Bugs: CLOUDSTACK-4967
    https://issues.apache.org/jira/browse/CLOUDSTACK-4967


Repository: cloudstack-git


Description
-------

1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.

2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.

This patch may need some refinement, but I want to get it to the vxlan devs for review.


Diffs
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
  scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 

Diff: https://reviews.apache.org/r/15014/diff/


Testing
-------

I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.

This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.

Here they are running side by side:

root@devcloud-kvm-u:~# brctl show
bridge name	bridge id		STP enabled	interfaces
breth0-3905		8000.000c29d82947	no		eth0.3905
							vnet8
brvx-1213043		8000.a6e6026f7fbb	no		vnet9
							vxlan1213043
brvx-1589169		8000.663d4e875e78	no		vnet10
							vxlan1589169
cloud0		8000.fe00a9fe0069	no		vnet0
							vnet2
							vnet3
cloudbr0		8000.000c29d82947	no		eth0
							vnet4
							vnet6
cloudbr1		8000.000c29d82951	no		eth1
							vnet1
							vnet5
							vnet7

root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
    vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
    vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
    vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
root@devcloud-kvm-u:~/cloudstack# ip route show
default via 192.168.100.1 dev cloudbr1  metric 100
169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
239.18.130.115 dev eth2  scope link
239.24.63.177 dev eth2  scope link
239.25.0.110 dev eth2  scope link


Thanks,

Marcus Sorensen


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Yoshikazu Nojima <ma...@ynojima.net>.

> On Oct. 29, 2013, 6:26 p.m., Marcus Sorensen wrote:
> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java, line 164
> > <https://reviews.apache.org/r/15014/diff/3/?file=373389#file373389line164>
> >
> >     It is indeed traffic label if one exists. _pifs contains map "trafficLabel => physical nic", but also contains the general keys 'private' (for guest traffic type) and 'public' (for public traffic type) for default values. If you prefer we can change the variable name to "pifKey" in order to reduce confusion.
> >     
> >     If we pull the "_pifs.get(trafficLabel)" out of here, we have to add several lines or a new method just prior to every point where createVnetBr is called to see if we can get a nic, then check for physical interface if no mapping was found to pass it as nic.  I'm ok with doing a new method if you feel strongly that this needs to be split out. It would be better if it were within createVnetBr or wrapped createVnetBr so that one doesn't have to remember to call it every time prior to createVnetBr.
> 
> Yoshikazu Nojima wrote:
>     Previous comment is just a comment, not a strong one. Changing the 2nd param name to "pifKey" seems better for me.
> 
> Marcus Sorensen wrote:
>     Ok, I'll make that change and commit it.

Thanks, Marcus!


- Yoshikazu


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/#review27710
-----------------------------------------------------------


On Oct. 29, 2013, 5:51 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15014/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 5:51 p.m.)
> 
> 
> Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.
> 
> 
> Bugs: CLOUDSTACK-4967
>     https://issues.apache.org/jira/browse/CLOUDSTACK-4967
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> 1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.
> 
> 2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.
> 
> This patch may need some refinement, but I want to get it to the vxlan devs for review.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
>   scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
>   server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 
> 
> Diff: https://reviews.apache.org/r/15014/diff/
> 
> 
> Testing
> -------
> 
> I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.
> 
> This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.
> 
> Here they are running side by side:
> 
> root@devcloud-kvm-u:~# brctl show
> bridge name	bridge id		STP enabled	interfaces
> breth0-3905		8000.000c29d82947	no		eth0.3905
> 							vnet8
> brvx-1213043		8000.a6e6026f7fbb	no		vnet9
> 							vxlan1213043
> brvx-1589169		8000.663d4e875e78	no		vnet10
> 							vxlan1589169
> cloud0		8000.fe00a9fe0069	no		vnet0
> 							vnet2
> 							vnet3
> cloudbr0		8000.000c29d82947	no		eth0
> 							vnet4
> 							vnet6
> cloudbr1		8000.000c29d82951	no		eth1
> 							vnet1
> 							vnet5
> 							vnet7
> 
> root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
> 56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
>     vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
> 59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
>     vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
> 62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
>     vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
> root@devcloud-kvm-u:~/cloudstack# ip route show
> default via 192.168.100.1 dev cloudbr1  metric 100
> 169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
> 172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
> 192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
> 239.18.130.115 dev eth2  scope link
> 239.24.63.177 dev eth2  scope link
> 239.25.0.110 dev eth2  scope link
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Yoshikazu Nojima <ma...@ynojima.net>.

> On Oct. 29, 2013, 6:26 p.m., Marcus Sorensen wrote:
> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java, line 164
> > <https://reviews.apache.org/r/15014/diff/3/?file=373389#file373389line164>
> >
> >     It is indeed traffic label if one exists. _pifs contains map "trafficLabel => physical nic", but also contains the general keys 'private' (for guest traffic type) and 'public' (for public traffic type) for default values. If you prefer we can change the variable name to "pifKey" in order to reduce confusion.
> >     
> >     If we pull the "_pifs.get(trafficLabel)" out of here, we have to add several lines or a new method just prior to every point where createVnetBr is called to see if we can get a nic, then check for physical interface if no mapping was found to pass it as nic.  I'm ok with doing a new method if you feel strongly that this needs to be split out. It would be better if it were within createVnetBr or wrapped createVnetBr so that one doesn't have to remember to call it every time prior to createVnetBr.

Previous comment is just a comment, not a strong one. Changing the 2nd param name to "pifKey" seems better for me.


- Yoshikazu


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/#review27710
-----------------------------------------------------------


On Oct. 29, 2013, 5:51 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15014/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 5:51 p.m.)
> 
> 
> Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.
> 
> 
> Bugs: CLOUDSTACK-4967
>     https://issues.apache.org/jira/browse/CLOUDSTACK-4967
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> 1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.
> 
> 2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.
> 
> This patch may need some refinement, but I want to get it to the vxlan devs for review.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
>   scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
>   server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 
> 
> Diff: https://reviews.apache.org/r/15014/diff/
> 
> 
> Testing
> -------
> 
> I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.
> 
> This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.
> 
> Here they are running side by side:
> 
> root@devcloud-kvm-u:~# brctl show
> bridge name	bridge id		STP enabled	interfaces
> breth0-3905		8000.000c29d82947	no		eth0.3905
> 							vnet8
> brvx-1213043		8000.a6e6026f7fbb	no		vnet9
> 							vxlan1213043
> brvx-1589169		8000.663d4e875e78	no		vnet10
> 							vxlan1589169
> cloud0		8000.fe00a9fe0069	no		vnet0
> 							vnet2
> 							vnet3
> cloudbr0		8000.000c29d82947	no		eth0
> 							vnet4
> 							vnet6
> cloudbr1		8000.000c29d82951	no		eth1
> 							vnet1
> 							vnet5
> 							vnet7
> 
> root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
> 56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
>     vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
> 59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
>     vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
> 62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
>     vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
> root@devcloud-kvm-u:~/cloudstack# ip route show
> default via 192.168.100.1 dev cloudbr1  metric 100
> 169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
> 172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
> 192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
> 239.18.130.115 dev eth2  scope link
> 239.24.63.177 dev eth2  scope link
> 239.25.0.110 dev eth2  scope link
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Marcus Sorensen <sh...@gmail.com>.

> On Oct. 29, 2013, 6:26 p.m., Marcus Sorensen wrote:
> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java, line 164
> > <https://reviews.apache.org/r/15014/diff/3/?file=373389#file373389line164>
> >
> >     It is indeed traffic label if one exists. _pifs contains map "trafficLabel => physical nic", but also contains the general keys 'private' (for guest traffic type) and 'public' (for public traffic type) for default values. If you prefer we can change the variable name to "pifKey" in order to reduce confusion.
> >     
> >     If we pull the "_pifs.get(trafficLabel)" out of here, we have to add several lines or a new method just prior to every point where createVnetBr is called to see if we can get a nic, then check for physical interface if no mapping was found to pass it as nic.  I'm ok with doing a new method if you feel strongly that this needs to be split out. It would be better if it were within createVnetBr or wrapped createVnetBr so that one doesn't have to remember to call it every time prior to createVnetBr.
> 
> Yoshikazu Nojima wrote:
>     Previous comment is just a comment, not a strong one. Changing the 2nd param name to "pifKey" seems better for me.

Ok, I'll make that change and commit it.


- Marcus


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/#review27710
-----------------------------------------------------------


On Oct. 29, 2013, 5:51 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15014/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 5:51 p.m.)
> 
> 
> Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.
> 
> 
> Bugs: CLOUDSTACK-4967
>     https://issues.apache.org/jira/browse/CLOUDSTACK-4967
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> 1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.
> 
> 2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.
> 
> This patch may need some refinement, but I want to get it to the vxlan devs for review.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
>   scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
>   server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 
> 
> Diff: https://reviews.apache.org/r/15014/diff/
> 
> 
> Testing
> -------
> 
> I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.
> 
> This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.
> 
> Here they are running side by side:
> 
> root@devcloud-kvm-u:~# brctl show
> bridge name	bridge id		STP enabled	interfaces
> breth0-3905		8000.000c29d82947	no		eth0.3905
> 							vnet8
> brvx-1213043		8000.a6e6026f7fbb	no		vnet9
> 							vxlan1213043
> brvx-1589169		8000.663d4e875e78	no		vnet10
> 							vxlan1589169
> cloud0		8000.fe00a9fe0069	no		vnet0
> 							vnet2
> 							vnet3
> cloudbr0		8000.000c29d82947	no		eth0
> 							vnet4
> 							vnet6
> cloudbr1		8000.000c29d82951	no		eth1
> 							vnet1
> 							vnet5
> 							vnet7
> 
> root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
> 56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
>     vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
> 59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
>     vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
> 62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
>     vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
> root@devcloud-kvm-u:~/cloudstack# ip route show
> default via 192.168.100.1 dev cloudbr1  metric 100
> 169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
> 172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
> 192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
> 239.18.130.115 dev eth2  scope link
> 239.24.63.177 dev eth2  scope link
> 239.25.0.110 dev eth2  scope link
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/#review27710
-----------------------------------------------------------



plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
<https://reviews.apache.org/r/15014/#comment53795>

    It is indeed traffic label if one exists. _pifs contains map "trafficLabel => physical nic", but also contains the general keys 'private' (for guest traffic type) and 'public' (for public traffic type) for default values. If you prefer we can change the variable name to "pifKey" in order to reduce confusion.
    
    If we pull the "_pifs.get(trafficLabel)" out of here, we have to add several lines or a new method just prior to every point where createVnetBr is called to see if we can get a nic, then check for physical interface if no mapping was found to pass it as nic.  I'm ok with doing a new method if you feel strongly that this needs to be split out. It would be better if it were within createVnetBr or wrapped createVnetBr so that one doesn't have to remember to call it every time prior to createVnetBr.


- Marcus Sorensen


On Oct. 29, 2013, 5:51 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15014/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 5:51 p.m.)
> 
> 
> Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.
> 
> 
> Bugs: CLOUDSTACK-4967
>     https://issues.apache.org/jira/browse/CLOUDSTACK-4967
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> 1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.
> 
> 2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.
> 
> This patch may need some refinement, but I want to get it to the vxlan devs for review.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
>   scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
>   server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 
> 
> Diff: https://reviews.apache.org/r/15014/diff/
> 
> 
> Testing
> -------
> 
> I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.
> 
> This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.
> 
> Here they are running side by side:
> 
> root@devcloud-kvm-u:~# brctl show
> bridge name	bridge id		STP enabled	interfaces
> breth0-3905		8000.000c29d82947	no		eth0.3905
> 							vnet8
> brvx-1213043		8000.a6e6026f7fbb	no		vnet9
> 							vxlan1213043
> brvx-1589169		8000.663d4e875e78	no		vnet10
> 							vxlan1589169
> cloud0		8000.fe00a9fe0069	no		vnet0
> 							vnet2
> 							vnet3
> cloudbr0		8000.000c29d82947	no		eth0
> 							vnet4
> 							vnet6
> cloudbr1		8000.000c29d82951	no		eth1
> 							vnet1
> 							vnet5
> 							vnet7
> 
> root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
> 56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
>     vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
> 59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
>     vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
> 62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
>     vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
> root@devcloud-kvm-u:~/cloudstack# ip route show
> default via 192.168.100.1 dev cloudbr1  metric 100
> 169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
> 172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
> 192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
> 239.18.130.115 dev eth2  scope link
> 239.24.63.177 dev eth2  scope link
> 239.25.0.110 dev eth2  scope link
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Yoshikazu Nojima <ma...@ynojima.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/#review27708
-----------------------------------------------------------



plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
<https://reviews.apache.org/r/15014/#comment53793>

    I feel odds with the parameter name 'trafficLabel'.
    In L.135, you specify "public" to this parameter, but is it trafficLabel?
    I'm not familiar with CloudStack terminology, but in my understanding, it is trafic type.
    
    I think leaving 2nd parameter "nic" and making pif lookup logic outside of this method are better. 
    
    How do you think?
    
    


- Yoshikazu Nojima


On Oct. 29, 2013, 5:51 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15014/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 5:51 p.m.)
> 
> 
> Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.
> 
> 
> Bugs: CLOUDSTACK-4967
>     https://issues.apache.org/jira/browse/CLOUDSTACK-4967
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> 1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.
> 
> 2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.
> 
> This patch may need some refinement, but I want to get it to the vxlan devs for review.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
>   scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
>   server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 
> 
> Diff: https://reviews.apache.org/r/15014/diff/
> 
> 
> Testing
> -------
> 
> I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.
> 
> This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.
> 
> Here they are running side by side:
> 
> root@devcloud-kvm-u:~# brctl show
> bridge name	bridge id		STP enabled	interfaces
> breth0-3905		8000.000c29d82947	no		eth0.3905
> 							vnet8
> brvx-1213043		8000.a6e6026f7fbb	no		vnet9
> 							vxlan1213043
> brvx-1589169		8000.663d4e875e78	no		vnet10
> 							vxlan1589169
> cloud0		8000.fe00a9fe0069	no		vnet0
> 							vnet2
> 							vnet3
> cloudbr0		8000.000c29d82947	no		eth0
> 							vnet4
> 							vnet6
> cloudbr1		8000.000c29d82951	no		eth1
> 							vnet1
> 							vnet5
> 							vnet7
> 
> root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
> 56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
>     vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
> 59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
>     vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
> 62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
>     vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
> root@devcloud-kvm-u:~/cloudstack# ip route show
> default via 192.168.100.1 dev cloudbr1  metric 100
> 169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
> 172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
> 192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
> 239.18.130.115 dev eth2  scope link
> 239.24.63.177 dev eth2  scope link
> 239.25.0.110 dev eth2  scope link
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/
-----------------------------------------------------------

(Updated Oct. 29, 2013, 5:51 p.m.)


Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.


Changes
-------

Fix mangled patch formatting.


Bugs: CLOUDSTACK-4967
    https://issues.apache.org/jira/browse/CLOUDSTACK-4967


Repository: cloudstack-git


Description
-------

1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.

2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.

This patch may need some refinement, but I want to get it to the vxlan devs for review.


Diffs (updated)
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
  scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
  server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 

Diff: https://reviews.apache.org/r/15014/diff/


Testing
-------

I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.

This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.

Here they are running side by side:

root@devcloud-kvm-u:~# brctl show
bridge name	bridge id		STP enabled	interfaces
breth0-3905		8000.000c29d82947	no		eth0.3905
							vnet8
brvx-1213043		8000.a6e6026f7fbb	no		vnet9
							vxlan1213043
brvx-1589169		8000.663d4e875e78	no		vnet10
							vxlan1589169
cloud0		8000.fe00a9fe0069	no		vnet0
							vnet2
							vnet3
cloudbr0		8000.000c29d82947	no		eth0
							vnet4
							vnet6
cloudbr1		8000.000c29d82951	no		eth1
							vnet1
							vnet5
							vnet7

root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
    vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
    vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
    vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
root@devcloud-kvm-u:~/cloudstack# ip route show
default via 192.168.100.1 dev cloudbr1  metric 100
169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
239.18.130.115 dev eth2  scope link
239.24.63.177 dev eth2  scope link
239.25.0.110 dev eth2  scope link


Thanks,

Marcus Sorensen


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Yoshikazu Nojima <ma...@ynojima.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/#review27701
-----------------------------------------------------------



server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/15014/#comment53789>

    '");' seems gone.


- Yoshikazu Nojima


On Oct. 29, 2013, 4:48 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15014/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 4:48 p.m.)
> 
> 
> Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.
> 
> 
> Bugs: CLOUDSTACK-4967
>     https://issues.apache.org/jira/browse/CLOUDSTACK-4967
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> 1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.
> 
> 2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.
> 
> This patch may need some refinement, but I want to get it to the vxlan devs for review.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
>   scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
>   server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 
> 
> Diff: https://reviews.apache.org/r/15014/diff/
> 
> 
> Testing
> -------
> 
> I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.
> 
> This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.
> 
> Here they are running side by side:
> 
> root@devcloud-kvm-u:~# brctl show
> bridge name	bridge id		STP enabled	interfaces
> breth0-3905		8000.000c29d82947	no		eth0.3905
> 							vnet8
> brvx-1213043		8000.a6e6026f7fbb	no		vnet9
> 							vxlan1213043
> brvx-1589169		8000.663d4e875e78	no		vnet10
> 							vxlan1589169
> cloud0		8000.fe00a9fe0069	no		vnet0
> 							vnet2
> 							vnet3
> cloudbr0		8000.000c29d82947	no		eth0
> 							vnet4
> 							vnet6
> cloudbr1		8000.000c29d82951	no		eth1
> 							vnet1
> 							vnet5
> 							vnet7
> 
> root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
> 56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
>     vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
> 59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
>     vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
> 62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
>     vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
> root@devcloud-kvm-u:~/cloudstack# ip route show
> default via 192.168.100.1 dev cloudbr1  metric 100
> 169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
> 172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
> 192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
> 239.18.130.115 dev eth2  scope link
> 239.24.63.177 dev eth2  scope link
> 239.25.0.110 dev eth2  scope link
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request 15014: allow physical devices as guest traffic label, vxlan bridge name fix

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15014/
-----------------------------------------------------------

(Updated Oct. 29, 2013, 4:48 p.m.)


Review request for cloudstack, Toshiaki Hatano and Yoshikazu Nojima.


Changes
-------

Updated to validate VNI range isn't used on a different physical network in zone. Tested with VNI 3000-3500 on guest eth0 and VNI 3500-4000 on guest eth2. This failed with "VNI 3500 already exists on another network in zone, please specify a unique range". Changing physical net eth2's range to 3501-4000 was success. Editing ranges similarly throws the error.


Bugs: CLOUDSTACK-4967
    https://issues.apache.org/jira/browse/CLOUDSTACK-4967


Repository: cloudstack-git


Description
-------

1) vxlan will use bridge scheme 'brvx-<vni>'. Multiple physical networks can host guest traffic type with vxlan isolation, so long as they don't use the same VNI range.

2) Guest traffic labels can be physical interface if bridge by given name is not found. Normally we take traffic label name, find the matching bridge, then resolve that to a physical interface. Then we create guest bridges on that interface. Now we can just specify the interface.

This patch may need some refinement, but I want to get it to the vxlan devs for review.


Diffs (updated)
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java f945b74 
  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 286d0f7 
  scripts/vm/network/vnet/modifyvxlan.sh a3ec71f 
  server/src/com/cloud/network/NetworkServiceImpl.java cf419f3 

Diff: https://reviews.apache.org/r/15014/diff/


Testing
-------

I created two physical networks hosting guest traffic, one as a vlan isolation type, the other a vxlan isolation type. The vlan isolation type has traffic label 'cloudbr0', on physical eth0, and the vxlan has traffic label 'eth2' on physical network eth2.

This patch would still allow multiple guest networks to use vxlan isolation, just not the same VNI numbers. It doesn't enforce this though.

Here they are running side by side:

root@devcloud-kvm-u:~# brctl show
bridge name	bridge id		STP enabled	interfaces
breth0-3905		8000.000c29d82947	no		eth0.3905
							vnet8
brvx-1213043		8000.a6e6026f7fbb	no		vnet9
							vxlan1213043
brvx-1589169		8000.663d4e875e78	no		vnet10
							vxlan1589169
cloud0		8000.fe00a9fe0069	no		vnet0
							vnet2
							vnet3
cloudbr0		8000.000c29d82947	no		eth0
							vnet4
							vnet6
cloudbr1		8000.000c29d82951	no		eth1
							vnet1
							vnet5
							vnet7

root@devcloud-kvm-u:~/cloudstack# ip -d link show | grep vxlan
56: vxlan1213043: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1213043 state UNKNOWN mode DEFAULT group default
    vxlan id 1213043 group 239.18.130.115 dev eth2 port 32768 61000 ttl 10 ageing 300
59: vxlan1589169: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1589169 state UNKNOWN mode DEFAULT group default
    vxlan id 1589169 group 239.24.63.177 dev eth2 port 32768 61000 ttl 10 ageing 300
62: vxlan1638510: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue master brvx-1638510 state UNKNOWN mode DEFAULT group default
    vxlan id 1638510 group 239.25.0.110 dev eth2 port 32768 61000 ttl 10 ageing 300
root@devcloud-kvm-u:~/cloudstack# ip route show
default via 192.168.100.1 dev cloudbr1  metric 100
169.254.0.0/16 dev cloud0  proto kernel  scope link  src 169.254.0.1
172.17.10.0/24 dev cloudbr0  proto kernel  scope link  src 172.17.10.10
192.168.100.0/24 dev cloudbr1  proto kernel  scope link  src 192.168.100.10
239.18.130.115 dev eth2  scope link
239.24.63.177 dev eth2  scope link
239.25.0.110 dev eth2  scope link


Thanks,

Marcus Sorensen