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

Set the queue size for Multiqueue virtio-net as the number of vCPUs on the guest. #719

Merged
merged 2 commits into from
Sep 13, 2018

Conversation

amshinde
Copy link
Member

Instead of using a default queue size of 8 for Multiqueue virtio-net,
use the number of vCPUs on the guest as the queue size.
This is the recommended approach for setting up the queue size.
This also shown better performance results.

This api will allow the config to be accessed by other subsystems
such as network.

Signed-off-by: Archana Shinde <[email protected]>
@egernst egernst added the review label Sep 12, 2018
Instead of using a default queue size of 8 for macvtap fds,
use the number of CPUs on the guest as the queue size.
This is the recommended approach. This also shown better
performance results.

Fixes kata-containers#680

Signed-off-by: Archana Shinde <[email protected]>
@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 169193 KB
Proxy: 4109 KB
Shim: 8983 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2006580 KB

@sboeuf sboeuf added the enhancement Improvement to an existing feature label Sep 12, 2018
Copy link

@sboeuf sboeuf left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@opendev-zuul
Copy link

opendev-zuul bot commented Sep 12, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@amshinde amshinde requested a review from egernst September 12, 2018 21:04
@@ -226,7 +225,7 @@ func networkLogger() *logrus.Entry {
func (endpoint *VirtualEndpoint) Attach(h hypervisor) error {
networkLogger().WithField("endpoint-type", "virtual").Info("Attaching endpoint")

if err := xconnectVMNetwork(&(endpoint.NetPair), true); err != nil {
if err := xconnectVMNetwork(&(endpoint.NetPair), true, h.hypervisorConfig().NumVCPUs); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Do we care about hotplugged CPUs? Properly not since they might be unplugged later. Just want to know your opinions about it.

Copy link
Member

Choose a reason for hiding this comment

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

Hey @bergwolf - under what use-cases would we be actively hotplugging/unplugging?

Copy link
Member

Choose a reason for hiding this comment

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

@egernst Currently no. I don't know any use case that would require removing a container while having other containers still running in the same sandbox. It is just that our APIs and CLI interfaces allow such behavior. nvm. It's fine with me to just use the default CPU numbers for the queue size.

Copy link
Member

Choose a reason for hiding this comment

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

Well, I think we do care about the resulting number of vCPUS. This shouldn't default to the default CPUs, but to what we request.

@amshinde @sboeuf

@bergwolf
Copy link
Member

bergwolf commented Sep 13, 2018

LGTM!

Approved with PullApprove

@bergwolf bergwolf merged commit 5404aab into kata-containers:master Sep 13, 2018
@egernst egernst removed the review label Sep 13, 2018
@amshinde amshinde deleted the net-queue-size branch July 11, 2019 22:28
egernst pushed a commit to egernst/runtime that referenced this pull request Feb 9, 2021
This includes fix for CVE-2019-19921

3291d66b rootfs: do not permit /proc mounts to non-directories
55f8c254 temporarily disable CRIU tests
5c20ea14 fix merging kata-containers#2177 and kata-containers#2169
8541d9cf Fix race checking for process exit and waiting for exec fifo
52951a7c Fix race in tty integration test with slow startup
8ddd8920 libcontainer: add method to get cgroup config from cgroup
Manager
cd7c59d0 libcontainer: export createCgroupConfig
ec49f98d fs2: support legacy device spec (to pass CI)
88e8350d cgroup2: split fs2 from fs
41a20b58 Expose network interfaces via runc events
48b055c4 Makefile: allow overriding `docker` command
42690e68 Make event types public
faf1e44e cgroup2: ebpf: increase RLIM_MEMLOCK to avoid BPF_PROG_LOAD
error
ccd4436f .travis.yml: add Fedora 31 vagrant box (for cgroup2)
faf673ee cgroup2: port over eBPF device controller from crun
74a3fe5d cgroup2: do not parse /proc/cgroups
9c81440f cgroup2: allow mounting /sys/fs/cgroup in UserNS without
unsharing CgroupNS
13919f5d Remove the static_build build tag.
dbd771e4 cgroup2: implement `runc ps`
9996cf7d README.md: clarify cgroup2 support is not ready for production
d918e7f4 cpuset_v2: skip Apply when no limit is specified
033936ef io_v2.go: remove blkio v1 code
a610a848 criu: Ensure other users cannot read c/r files
b28f58f3 Set unified mountpoint in find mnt func
f017e0f9 checkpoint: Set descriptors.json file mode to 0600
4be50fe3 SECURITY: Add Security Policy
2111613c VERSION: back to development
28e58a0f Support different field counts of cpuaact.stats
e63b797f Handle ENODEV when accessing the freezer.state file
5e0e67d7 fix permission denied
056909bd Adds note about user ns for rootless containers

Fixes kata-containers#719

Signed-off-by: Archana Shinde <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants