virtcontainers: network: Add VFIO hotplug support #620

sboeuf · 2018-08-21T19:31:17Z

In order to support full network hotplug, it's mandatory that we can
support the hotplug of a VFIO device for the network interface.

Fixes #619

Signed-off-by: Sebastien Boeuf [email protected]

sboeuf · 2018-08-21T19:38:22Z

@amshinde PTAL

amshinde · 2018-08-21T19:42:28Z

virtcontainers/network.go

@@ -417,7 +417,29 @@ func (endpoint *PhysicalEndpoint) Detach(netNsCreated bool, netNsPath string) er

 // HotAttach for physical endpoint not supported yet
 func (endpoint *PhysicalEndpoint) HotAttach(h hypervisor) error {
-	return fmt.Errorf("PhysicalEndpoint does not support Hot attach")
+	networkLogger().WithField("endpoint-type", "physical").Info("Attaching endpoint (hotplug)")


You will need to implement HotDetach as well for physical interface.

Implementing HotDetach() needs some more rework. It needs the ID of the device to be stored, but the structures don't provide this ability. The hot detach requires more work, and can be handled in a follow up PR.

opendev-zuul · 2018-08-21T21:07:49Z

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

kata-runsh : TIMED_OUT in 1h 01m 22s

katacontainersbot · 2018-08-21T21:17:02Z

PSS Measurement:
Qemu: 166119 KB
Proxy: 4066 KB
Shim: 8788 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003192 KB

bergwolf

LTGM with just one nit.

bergwolf · 2018-08-23T04:31:26Z

virtcontainers/network.go

@@ -417,7 +417,29 @@ func (endpoint *PhysicalEndpoint) Detach(netNsCreated bool, netNsPath string) er

 // HotAttach for physical endpoint not supported yet


This comment is going to be obsolete with this PR :)

WeiZhang555 · 2018-08-23T06:11:38Z

virtcontainers/network.go

+		BDF: endpoint.BDF,
+	}
+
+	_, err = h.hotplugAddDevice(d, vfioDev)


Since you are doing the rework, how about we use sandbox.DeviceManager to manage vfio devices?

e.g.

dev, err := sandbox.DeviceManager.NewDevice(deviceInfo{}) sandbox.DeviceManager.AttachDevice(dev.DeviceID(), sandbox)

This can gurantee we manage devices with same struct and after container exits, every devices can be detached correctly.

This will also need some rework as PhysicalEndpoint doesn't have a sandbox field

@WeiZhang555 I wanted to enable VFIO net hotplug support in order to support #600 but now that I have posted #623 , I think I can spend more time to actually do this PR more properly.
I'll try to spend some time to use the DeviceManager, so that I can implement the detach too ;)

sboeuf · 2018-08-23T15:08:05Z

Just added do-not-merge label, as this PR should use the DeviceManager model to be able to detach devices.

sboeuf · 2018-09-19T07:32:07Z

Need #758 to be fixed first.

jodh-intel · 2018-09-28T10:30:51Z

#758 has now landed :)

jodh-intel · 2018-09-28T10:31:28Z

virtcontainers/network.go

+	}
+	id := hex.EncodeToString(randBytes)
+
+	// TODO: use device manager as general device management entrance


Could you make this an issue or we'll forget ;)

amshinde · 2018-10-08T19:40:32Z

@sboeuf Any updates?
Also, I think its fine not to use the Devicemanager for now, since I think its cleaner to keep the network devices separate from sandbox devices.

sboeuf · 2018-10-08T19:44:23Z

@amshinde if we do so, we wont have the hot-detach implementation. I am fine either way.

raravena80 · 2018-10-22T16:37:45Z

@sboeuf ping (From your weekly Kata herder)

raravena80 · 2018-11-19T17:56:42Z

@sboeuf nudge.

raravena80 · 2018-12-18T05:18:31Z

@sboeuf @amshinde are we merging this eventually?

jodh-intel · 2019-01-07T17:50:48Z

@sboeuf - another conflicted branch.

In order to support full network hotplug, it's mandatory that we can support the hotplug of a VFIO device for the network interface. Fixes kata-containers#619 Signed-off-by: Sebastien Boeuf <[email protected]>

sboeuf · 2019-01-08T11:31:46Z

@jodh-intel rebased!

sboeuf · 2019-01-08T11:38:17Z

/test

jodh-intel · 2019-01-08T11:53:54Z

virtcontainers/physical_endpoint.go

+	}
+	id := hex.EncodeToString(randBytes)
+
+	// TODO: use device manager as general device management entrance


Could you remove, or add an issue URL here maybe?

jodh-intel · 2019-01-08T11:55:46Z

virtcontainers/physical_endpoint.go

 func (endpoint *PhysicalEndpoint) HotAttach(h hypervisor) error {
-	return fmt.Errorf("PhysicalEndpoint does not support Hot attach")
+	networkLogger().WithField("endpoint-type", "physical").Info("Attaching endpoint (hotplug)")


Suggestion:

Suggested change

networkLogger().WithField("endpoint-type", "physical").Info("Attaching endpoint (hotplug)")

networkLogger().WithField("endpoint-type", "physical").Info("Hot attaching endpoint")

Alternatively, add a field for hotplug.

jodh-intel · 2019-01-08T11:57:15Z

virtcontainers/physical_endpoint.go

@@ -98,9 +100,31 @@ func (endpoint *PhysicalEndpoint) Detach(netNsCreated bool, netNsPath string) er
 	return bindNICToHost(endpoint)
 }

-// HotAttach for physical endpoint not supported yet
+// HotAttach for physical endpoint


What about HotDetach()?

jodh-intel · 2019-01-23T13:59:42Z

/me blows the dust off and pings @sboeuf...

caoruidong · 2019-01-30T08:46:14Z

/retest

raravena80 · 2019-02-04T20:38:46Z

@sboeuf @caoruidong ping

jodh-intel · 2019-02-26T11:16:28Z

Another August '18 vintage PR. @sboeuf - what's the plan here?

sboeuf · 2019-02-26T14:50:31Z

Same here, let's close and get back to it if really needed.

When agent runs as init, it is not able to read `/proc/self/cmdline` because of `/proc` has not been mounted when the C constructor is called. This reverts commit cfbd8c9. fixes kata-containers#620 Signed-off-by: Julio Montes <[email protected]>

egernst added the review label Aug 21, 2018

sboeuf requested a review from amshinde August 21, 2018 19:32

amshinde reviewed Aug 21, 2018

View reviewed changes

bergwolf approved these changes Aug 23, 2018

View reviewed changes

WeiZhang555 reviewed Aug 23, 2018

View reviewed changes

sboeuf added the do-not-merge label Aug 23, 2018

sboeuf added the feature New functionality label Sep 12, 2018

jodh-intel reviewed Sep 28, 2018

View reviewed changes

virtcontainers: network: Add VFIO hotplug support

e323d99

In order to support full network hotplug, it's mandatory that we can support the hotplug of a VFIO device for the network interface. Fixes kata-containers#619 Signed-off-by: Sebastien Boeuf <[email protected]>

sboeuf force-pushed the vfio_hp branch from ff604e4 to e323d99 Compare January 8, 2019 11:30

sboeuf removed the do-not-merge label Jan 8, 2019

jodh-intel reviewed Jan 8, 2019

View reviewed changes

sboeuf closed this Feb 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

virtcontainers: network: Add VFIO hotplug support #620

virtcontainers: network: Add VFIO hotplug support #620

sboeuf commented Aug 21, 2018

sboeuf commented Aug 21, 2018

amshinde Aug 21, 2018

sboeuf Aug 21, 2018

opendev-zuul bot commented Aug 21, 2018

katacontainersbot commented Aug 21, 2018

bergwolf left a comment

bergwolf Aug 23, 2018

WeiZhang555 Aug 23, 2018

sboeuf Aug 23, 2018

sboeuf commented Aug 23, 2018

sboeuf commented Sep 19, 2018

jodh-intel commented Sep 28, 2018

jodh-intel Sep 28, 2018

amshinde commented Oct 8, 2018

sboeuf commented Oct 8, 2018

raravena80 commented Oct 22, 2018

raravena80 commented Nov 19, 2018

raravena80 commented Dec 18, 2018

jodh-intel commented Jan 7, 2019

sboeuf commented Jan 8, 2019

sboeuf commented Jan 8, 2019

jodh-intel Jan 8, 2019

jodh-intel Jan 8, 2019

jodh-intel Jan 8, 2019

jodh-intel commented Jan 23, 2019

caoruidong commented Jan 30, 2019

raravena80 commented Feb 4, 2019 •

edited

Loading

jodh-intel commented Feb 26, 2019

sboeuf commented Feb 26, 2019

		@@ -417,7 +417,29 @@ func (endpoint *PhysicalEndpoint) Detach(netNsCreated bool, netNsPath string) er

		// HotAttach for physical endpoint not supported yet

	networkLogger().WithField("endpoint-type", "physical").Info("Attaching endpoint (hotplug)")
	networkLogger().WithField("endpoint-type", "physical").Info("Hot attaching endpoint")

virtcontainers: network: Add VFIO hotplug support #620

virtcontainers: network: Add VFIO hotplug support #620

Conversation

sboeuf commented Aug 21, 2018

sboeuf commented Aug 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opendev-zuul bot commented Aug 21, 2018

katacontainersbot commented Aug 21, 2018

bergwolf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sboeuf commented Aug 23, 2018

sboeuf commented Sep 19, 2018

jodh-intel commented Sep 28, 2018

Choose a reason for hiding this comment

amshinde commented Oct 8, 2018

sboeuf commented Oct 8, 2018

raravena80 commented Oct 22, 2018

raravena80 commented Nov 19, 2018

raravena80 commented Dec 18, 2018

jodh-intel commented Jan 7, 2019

sboeuf commented Jan 8, 2019

sboeuf commented Jan 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jodh-intel commented Jan 23, 2019

caoruidong commented Jan 30, 2019

raravena80 commented Feb 4, 2019 • edited Loading

jodh-intel commented Feb 26, 2019

sboeuf commented Feb 26, 2019

raravena80 commented Feb 4, 2019 •

edited

Loading