Networking: Add support for multiqueue tap #475

mcastelino · 2017-11-09T23:18:34Z

Networking: Add support for multiqueue tap

Add support for multiqueue tap for the bridge mode.
This improves the multi-q performance of bridge mode.
However macvtap mode is still faster.

Fixes #247

Requires merge of vishvananda/netlink#293 followed by netlink revendoring

jodh-intel · 2017-11-10T10:10:21Z

aside from the conflict...

lgtm.

sboeuf · 2017-11-11T00:54:54Z

network.go

@@ -523,6 +524,35 @@ func createLink(netHandle *netlink.Handle, name string, expectedLink netlink.Lin
 	return getLinkByName(netHandle, name, expectedLink)
 }

+func createMqLink(netHandle *netlink.Handle, name string, expectedLink netlink.Link) (netlink.Link, []*os.File, error) {


Instead of adding this function, don't you think we could add a flag for the multiqueue to createLink():

func createLink(netHandle *netlink.Handle, name string, expectedLink netlink.Link, mq bool)

And because the return is different, we could simply ignore the list of FDs when not in multiqueue case.

@sboeuf I should have removed the old method completely. It is no longer needed. We will be switching to fd based methods for all networking interfaces as far as possible. This will give us more control on how we setup the interfaces. i.e. QEMU will not be performing any networking setup internally.

sboeuf

Few comments, but looks good.

sboeuf · 2017-11-11T00:57:14Z

network.go

+		return nil, fds, fmt.Errorf("LinkAdd() failed for %s name %s: %s", expectedLink.Type(), name, err)
+	}
+
+	tuntapLink, ok := newLink.(*netlink.Tuntap)


This is not consistent with the fact you want something generic by providing expectedLink type, but on the other hand, you cast this object into a *netlink.Tuntap whatever the expected link type is.

@sboeuf Good catch will fix

@sboeuf fixed

sboeuf · 2017-11-11T01:23:21Z

qemu.go

@@ -313,7 +313,7 @@ func (q *qemu) appendSocket(devices []ciaoQemu.Device, socket Socket) []ciaoQemu
 func networkModelToQemuType(model NetInterworkingModel) ciaoQemu.NetDeviceType {
 	switch model {
 	case ModelBridged:
-		return ciaoQemu.TAP
+		return ciaoQemu.MACVTAP


Why did you change this ?

@sboeuf The mq link uses the same connection methodology as macvtap. Hence the change. Longer term I should change the name of the ciaoQEMU type should be ciaoQemu.FD. macvtap was too specific as it is just a fd based networking connection.

I will rename/rework the qemu networking code quite a bit when adding support for vhost-user and macvlan and tc based connectivity. Will do the renaming as part of that just to avoid revendoring qemu.

mcastelino

@sboeuf did a minor rewrite. please review

mcastelino · 2017-11-17T23:40:48Z

network.go

+		return nil, fds, fmt.Errorf("LinkAdd() failed for %s name %s: %s", expectedLink.Type(), name, err)
+	}
+
+	tuntapLink, ok := newLink.(*netlink.Tuntap)


@sboeuf fixed

mcastelino · 2017-11-18T01:23:00Z

@chavafg can you look at why the CI is failing. There are two vendorings here. And somehow our CI is not picking them up properly

23:48:49 # github.com/clearcontainers/runtime/vendor/github.com/containers/virtcontainers
23:48:49 vendor/github.com/containers/virtcontainers/network.go:517: unknown field 'Queues' in struct literal of type netlink.Tuntap
23:48:49 vendor/github.com/containers/virtcontainers/network.go:518: undefined: netlink.TUNTAP_MULTI_QUEUE_DEFAULTS
23:48:49 vendor/github.com/containers/virtcontainers/network.go:546: tuntapLink.Fds undefined (type *netlink.Tuntap has no field or method Fds)
23:48:49 vendor/github.com/containers/virtcontainers/qemu.go:357: unknown field 'VhostFDs' in struct literal of type qemu.NetDevice

Revendor ciao to support multiqueue vhost 95b1aed Networking: Add vhost fd support bbed5b9 client: simplify returned type for quotas 9a7b502 client: split client library by API (& legacy) 171014b ciao-cli: Move client code to it's own package 5b228e3 ciao-cli: Ensure required fields and receivers are exported 692a492 ciao-cli: Split volume client and command handling 40d9fff ciao-cli: split client/command trace functionality 87b016b ciao-cli: Split client/command code for tenants a0683ae ciao-cli: split quotas functionality 489ebce ciao-cli: split node functionality into client and command code dd0ac20 ciao-cli: split workload functionality 4638327 ciao-cli: split instance functionality out 45f32a2 ciao-cli: create helper functions for GET/DELETE/POST and parsing 148cf25 ciao-cli: split client code out for image manipulation 4e555a6 ciao-cli: split client code for external IPs 9508a90 ciao-cli: split event code for list and delete 0a3a983 ciao-cli: with a single HTTP endpoint remove unnecessary API 545458d ciao-cli: Improve error handling in core client code 93f436a ciao-cli: remove scopedToken (residual from keystone) 3218b43 ciao-cli: ensure that the field named controllerURL really is a URL f7aa79c ciao-cli: validate that controllerURL and client certficate are set ac555b5 ciao-cli: move core client code to new struct Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>

Revendor netlink to support multiqueue c2a3de3 tuntap: Add multiqueue support e104583 Support num{tx,rx}queues and udp6zerocsum{tx,rx} bdf753e added support for Foo-over-UDP netlink calls aa48b8c Fix CalcRtable array parameter bug 63ca7e4 Support setting and retrieving route MTU/AdvMSS f7e518d add missing log import eb7ed87 Support LWTUNNEL_ENCAP_SEG6 6174cd8 Support invert in ip rules ae21927 Exclude linux specific test code from running on other platforms 09a4632 Properly use Skip() function, add -test.v when running tests a47a543 Allow to skip tests based on min kernel version required 71fa81e Expose xfrm state's current and window statistics c29ba20 added encapsulation attributes to Iptun added encapsulation attributes to Gretun implemented Sittun struct for supporting SIT tunnels 1272825 Fix index out of range when no metadata for gretap 0e3b74d replace syscall with golang.org/x/sys/unix Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>

amshinde

lgtm

Add support for multiqueue tap for the bridge mode. This improves the multi-q performance of bridge mode. However macvtap mode is still faster. Explicitly create vhost fd's for macvtap. This unifies the macvtap and tap implementations. Fixes containers#247 Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>

mcastelino · 2017-11-21T21:29:57Z

Closing this to reopen it using a different branch as the CI keeps failing due to vendoring.

sameo added the in progress label Nov 9, 2017

mcastelino requested review from sboeuf and amshinde November 9, 2017 23:19

sboeuf reviewed Nov 11, 2017

View reviewed changes

mcastelino force-pushed the topic/mqvhost branch from 632f683 to 0829996 Compare November 17, 2017 23:40

mcastelino commented Nov 17, 2017

View reviewed changes

mcastelino changed the title ~~WIP: Networking: Add support for multiqueue tap~~ Networking: Add support for multiqueue tap Nov 18, 2017

mcastelino added 2 commits November 20, 2017 15:12

mcastelino force-pushed the topic/mqvhost branch from 0829996 to 8fa3c35 Compare November 20, 2017 23:35

amshinde approved these changes Nov 20, 2017

View reviewed changes

mcastelino force-pushed the topic/mqvhost branch from 8fa3c35 to aa91b93 Compare November 21, 2017 00:39

mcastelino closed this Nov 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Networking: Add support for multiqueue tap #475

Networking: Add support for multiqueue tap #475

mcastelino commented Nov 9, 2017 •

edited by sameo

Loading

jodh-intel commented Nov 10, 2017 •

edited by dlespiau

Loading

sboeuf Nov 11, 2017 •

edited

Loading

mcastelino Nov 13, 2017

sboeuf left a comment

sboeuf Nov 11, 2017

mcastelino Nov 13, 2017

mcastelino Nov 17, 2017

sboeuf Nov 11, 2017

mcastelino Nov 13, 2017

mcastelino left a comment

mcastelino Nov 17, 2017

mcastelino commented Nov 18, 2017

amshinde left a comment

mcastelino commented Nov 21, 2017

Networking: Add support for multiqueue tap #475

Networking: Add support for multiqueue tap #475

Conversation

mcastelino commented Nov 9, 2017 • edited by sameo Loading

jodh-intel commented Nov 10, 2017 • edited by dlespiau Loading

sboeuf Nov 11, 2017 • edited Loading

Choose a reason for hiding this comment

mcastelino Nov 13, 2017

Choose a reason for hiding this comment

sboeuf left a comment

Choose a reason for hiding this comment

sboeuf Nov 11, 2017

Choose a reason for hiding this comment

mcastelino Nov 13, 2017

Choose a reason for hiding this comment

mcastelino Nov 17, 2017

Choose a reason for hiding this comment

sboeuf Nov 11, 2017

Choose a reason for hiding this comment

mcastelino Nov 13, 2017

Choose a reason for hiding this comment

mcastelino left a comment

Choose a reason for hiding this comment

mcastelino Nov 17, 2017

Choose a reason for hiding this comment

mcastelino commented Nov 18, 2017

amshinde left a comment

Choose a reason for hiding this comment

mcastelino commented Nov 21, 2017

mcastelino commented Nov 9, 2017 •

edited by sameo

Loading

jodh-intel commented Nov 10, 2017 •

edited by dlespiau

Loading

sboeuf Nov 11, 2017 •

edited

Loading