Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement binding strategy in creating and binding lport #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

huikang
Copy link
Collaborator

@huikang huikang commented Aug 26, 2016

  • see the guide: ansible/doc/binding-strategy.rst

Signed-off-by: Hui Kang kangh@us.ibm.com

@huikang
Copy link
Collaborator Author

huikang commented Aug 26, 2016

Address #95

Overview
========

In OVN, a logical port can be bind to a local OVS port on any chassis/hypervisor, depending on the VM scheduler (e.g., ``nova-scheduler``). The binding strategy potentially impacts the network performance. That is binding all logical ports in a logical network on a single hypervisor performs differently than distributing the ports on multiple hypervisors.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to linewrap this file at 80 characters. Can you do that please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mestery sorry about the wrong format. Please review the updated PR.

- see the guide: ansible/doc/binding-strategy.rst

Signed-off-by: Hui Kang <kangh@us.ibm.com>
class LogicalNetwork():
def __init__(self):
self.sandboxes = []
self.ports_per_network = 0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to add 'self.lswitch = None' in init(), otherwise get_lswitch() before set_lswitch() may raise a exception

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got you. Thanks.

@l8huang
Copy link
Collaborator

l8huang commented Aug 30, 2016

For distribute lswithes/ports to sandbox evenly, I run rally-ovs multiple times, each time with a set of sandbox, like:
rally-ovs task start create_and_bind_port.json --task-args '{"tor":"ToR-42", "start_cidr":"172.16.46.0/24"}'
rally-ovs task start create_and_bind_port.json --task-args '{"tor":"ToR-51", "start_cidr":"172.16.51.0/24"}'
rally-ovs task start create_and_bind_port.json --task-args '{"tor":"ToR-52", "start_cidr":"172.16.56.0/24"}'
....
What this change want to do is run rally-ovs only once with all sandboxes(fix me if I misunderstanding it), if it meets your requirement, it is ok to merge it.

@huikang
Copy link
Collaborator Author

huikang commented Aug 30, 2016

@l8huang your understanding is correct. The new variable network_per_sandbox I added in this PR determines how many logical network will be placed on each sandbox/chassis.

V2
- remove debug message
- address Lei's comment
@huikang
Copy link
Collaborator Author

huikang commented Aug 30, 2016

I will combine the commits into one when the PR is ready for merge. Thanks.

@huikang
Copy link
Collaborator Author

huikang commented Sep 17, 2016

@l8huang Just a gentle reminder: do you mind spending some minutes reviewing this PR? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants