Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

network: Don't assume multiple queues support by default #1028

Merged
merged 4 commits into from
Dec 17, 2018

Conversation

sboeuf
Copy link

@sboeuf sboeuf commented Dec 14, 2018

This pull request reorganizes slightly the network code to be able to check the hypervisor capabilities. A new hypervisor capabilities called multiQueueSupport has been added, and can be used to determine if the network should be setup with or without multiple queues.

Fixes #1027

Sebastien Boeuf added 4 commits December 14, 2018 14:50
In order to prevent from future duplication of calls into the
hypervisor interface, the hypervisor is directly passed as part
of the xConnectVMNetwork() function. Because this does not apply
the disconnection case, this commit splits the former function
into two separate ones.

Signed-off-by: Sebastien Boeuf <[email protected]>
The point of knowing the number of CPUs from the network perspective
is to determine the number of queues that can be allocated to the
network interface of the our virtual machine.

Therefore, it's more logical to name it queues from a network.go
perspective.

Signed-off-by: Sebastien Boeuf <[email protected]>
Each hypervisor is different and supports different options regarding
the network interface it creates. In particular, the multiqueue option
is not supported by Firecracker and should not be assumed by default.

Signed-off-by: Sebastien Boeuf <[email protected]>
…eues

In order to properly setup the network, hence allocate or not multiple
queues, this commit makes sure that the hypervisor capabilities are
checked for this.

Fixes kata-containers#1027

Signed-off-by: Sebastien Boeuf <[email protected]>
@sboeuf
Copy link
Author

sboeuf commented Dec 14, 2018

/cc @mcastelino @egernst @amshinde

@mcastelino
Copy link
Contributor

mcastelino commented Dec 14, 2018

LGTM

Approved with PullApprove

Copy link
Member

@egernst egernst left a comment

Choose a reason for hiding this comment

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

looks fine. reminds me we shoudl really adjust and put a limit on number of queues (ie, cap it after 4?)

@sboeuf
Copy link
Author

sboeuf commented Dec 15, 2018

/test

@sboeuf
Copy link
Author

sboeuf commented Dec 15, 2018

Let me fix the CI right now!

@sboeuf
Copy link
Author

sboeuf commented Dec 15, 2018

/test

@bergwolf
Copy link
Member

bergwolf commented Dec 17, 2018

LGTM. jenkins-ci-ubuntu-16-04-initrd failed on kata-containers/tests/issues/1010, restarting.

Approved with PullApprove

@egernst egernst merged commit 3148997 into kata-containers:master Dec 17, 2018
@grahamwhaley
Copy link
Contributor

@egernst - rather than hard code the queue cap limit (to 4 or whatever), add a toml config option to allow that to be set, and default that cap to 4 maybe.

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

Successfully merging this pull request may close these issues.

5 participants