updateInterface: enable hot-add nic on arm64 #545

jongwu · 2019-04-30T09:24:26Z

For now, update interface in agent will fail when hot-add
nic to a running containers on arm64 as rescan pci bus will
occur between uf and bf of shpc hotplug interrupt handling.

Another problem is that the rootBusPath will be
"/devices/platform/4010000000.pcie/pci0000:00" on arm64.

To enable hot-add nic on arm64, rootBusPath should be changed here
and shpc hotplug should be disabled in guest kernel.

This patch just change rootBusPath.

Fixes: #544
Signed-off-by: Jianyong Wu [email protected]

@bergwolf @jodh-intel @teawater @Pennyzct

jodh-intel · 2019-04-30T09:41:30Z

device_amd64.go

+
+var pciBusRescanFile = sysfsDir + "/bus/pci/rescan"
+
+func rescanPciBus() error {


Since hot-add differs only on ARM, how about keeping the new files (device_amd64.go, device_s390x.go, etc), but having all non-ARM implementations call a generic handler like this:

func rescanPciBus() error { return genericRescanPCIBus() }

That would avoid the duplication.

@jodh-intel thanks, if the implementation for rescanPciBus contains many lines of code, it is better to package them. but for only one line, keep it maybe better, as I fear of that more function call need more stack context switch which may be harmful to performance.

teawater · 2019-05-01T04:26:27Z

/test

jongwu · 2019-05-06T02:55:55Z

@teawater @bergwolf any comments?

jongwu · 2019-05-08T10:05:14Z

could someone help me retrigger test?

grahamwhaley · 2019-05-08T10:06:07Z

/test
sure

Pennyzct · 2019-05-08T10:08:02Z

Hi~ @grahamwhaley could we add ARM CI machine here?? ;)

grahamwhaley · 2019-05-08T10:15:24Z

@Pennyzct - I expect so. If you are confident it is stable enough, then I don't think anybody will object. The only 'rule' I try to lay down is that if it becomes unstable and starts to block PRs, and is not fixed in a reasonable time, then we may disable it again :-)
@chavafg - you OK with this?
So, @Pennyzct - we are also always looking for other members of the community to become helpers with the CI in general - would you like more access to the Jenkins master machine to help us with that? We should also discuss that ;-)

jodh-intel · 2019-05-08T10:29:39Z

@grahamwhaley - could you add those CI arch requirements maybe to https://github.com/kata-containers/ci/blob/master/README.md?

grahamwhaley · 2019-05-08T10:35:34Z

@jodh-intel - sure, I opened kata-containers/ci#156 as a starter.

chavafg · 2019-05-08T13:44:15Z

I agree we can add the arm job to the agent repo.

jodh-intel

I'm not a fan of duplication, but this "1-liner duplication" can always be refactored in a follow-up PR I guess.

Thanks for dealing with all the architectures here by the way too.

jodh-intel · 2019-05-10T10:23:01Z

/test

jongwu · 2019-05-13T09:30:34Z

@jodh-intel yeah, code duplication is not a good code style.

jongwu · 2019-05-14T01:19:54Z

@grahamwhaley @bergwolf any comments?

grahamwhaley · 2019-05-14T09:02:02Z

np from me. I'd quite like to hear from @devimc on it for the pci/hotplug stuff.
I nudged a rebuild of the jenkins-vsock - the only one failing, and I think that was a generic issue I'm hoping we fixed overnight...

jongwu · 2019-05-14T10:07:47Z

@grahamwhaley thanks.

jcvenegas · 2019-05-14T17:13:32Z

device_amd64.go

@@ -0,0 +1,11 @@
+//
+// Copyright (c) 2019 Intel Corporation


This is probably a wrong header

jcvenegas · 2019-05-14T17:13:52Z

device_ppc64le.go

@@ -0,0 +1,11 @@
+//
+// Copyright (c) 2019 Intel Corporation


ARM Limited?

@jcvenegas I'm confused with these copyright header, those no-arm64 code are all copy from the original file with Intel header. so I'm not sure how to write these header. @jodh-intel @grahamwhaley @bergwolf any idea?

Thanks for raising this issue @jongwu! I agree that this is confusing, so we need to document this. IANAL so will need to get input from folks more knowledgeable in that area. Since this file is based on an existing one, I would think it is "safe" to always reference the original copyright as well (so that you could list both Intel and ARM for 2019).

@grahamwhaley - any thoughts on this? I'm happy to write it up once we have clarity on this subtle area.

As to a location for where to put the words.. I think it would either need to go in:

https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md

But, for parity with https://github.com/kata-containers/documentation/blob/master/Documentation-Requirements.md, it may make sense to create a https://github.com/kata-containers/documentation/blob/master/Source-File-Requirements.md

Wherever it goes, it needs to cover the following copyright scenarios:

Adding a new source file.

Adding a new source file which is similar to an existing file:

Exact copy but for different arch (covering both a tiny fragment (like on this PR) and also a very large and complex file).

"Based on" an existing file but with potentially extensive modifications.

... ?

Thanks @jodh-intel, it is very helpful for me. If I get it right, the copyright header for device_amd64.go, device_ppc64le.go and device_s390x.go should include both intel and ARM. Is that right?
If that is the truth, what's the shape of these header? eg. it should be "Intel Corporation and ARM limited" or "Intel Corporation && ARM limited" or something else?

afaik, you would put your new addition copyright on a new line, so like:

// // Copyright (c) 2018 Intel Corporation // Copyright (c) 2019 <your company/person name here> // // SPDX-License-Identifier: Apache-2.0 //

and replace your company etc. :-)

I should probably note - your company may have its own ideas, rules or specific name/format for this btw, and you might want to check internally first.

// Copyright (c) 2019 ARM Limited
//
// SPDX-License-Identifier: Apache-2.0
//
plz use this. ;)

Thanks @grahamwhaley

For now, update interface in agent will fail when hot-add nic to a running containers on arm64 as rescan pci bus will occur between uf and bf of shpc hotplug interrupt handling. Another problem is that the rootBusPath will be "/devices/platform/4010000000.pcie/pci0000:00" on arm64. To enable hot-add nic on arm64, rootBusPath should be changed here and shpc hotplug should be disabled in guest kernel. This patch just change rootBusPath. Fixes: kata-containers#544 Signed-off-by: Jianyong Wu <[email protected]>

Pennyzct · 2019-05-16T01:56:44Z

Hi~ @grahamwhaley
Sorry for the delayed response about adding ARM CI machines on agent repo.
I got sidetracked for a few other issues. 😥
For now, there could only be me full-time dedicated to the ARM CI maintenance, in the future, there may be more arm folks.;) So if anytime ARM CI blocks PR here and doesn't get fixed for a quite time, you sure could bring it down.😄
And I'm very happy to become helpers with the CI community and have more access to the Jenkins master machine to help you guys.
So All in all, let's bring ARM CI online agent repo. ;)

grahamwhaley · 2019-05-16T09:17:56Z

@Pennyzct - as long as we are happy we have the capacity (as discussed on other Issue), sure - @chavafg maybe we should first add @Pennyzct to the Jenkins master as admin, and then guide on how to add (clone/edit) a new job... thx!

jongwu · 2019-05-17T10:42:39Z

@devimc any comments?

devimc · 2019-05-17T11:56:59Z

/test

jodh-intel reviewed Apr 30, 2019

View reviewed changes

grahamwhaley requested a review from amshinde April 30, 2019 09:45

jongwu force-pushed the nic branch 2 times, most recently from fb7d025 to 78a2fff Compare April 30, 2019 10:27

grahamwhaley requested a review from devimc May 3, 2019 15:52

jongwu force-pushed the nic branch from 78a2fff to 31b871a Compare May 7, 2019 03:40

jodh-intel approved these changes May 10, 2019

View reviewed changes

jcvenegas reviewed May 14, 2019

View reviewed changes

jongwu force-pushed the nic branch from 31b871a to 701344e Compare May 15, 2019 02:14

jongwu force-pushed the nic branch from 701344e to 5f9df74 Compare May 15, 2019 08:53

Pennyzct mentioned this pull request May 16, 2019

ci: Arm job queues getting large kata-containers/ci#159

Closed

devimc approved these changes May 17, 2019

View reviewed changes

teawater merged commit 065b34d into kata-containers:master May 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

updateInterface: enable hot-add nic on arm64 #545

updateInterface: enable hot-add nic on arm64 #545

jongwu commented Apr 30, 2019 •

edited

Loading

jodh-intel Apr 30, 2019

jongwu Apr 30, 2019

teawater commented May 1, 2019

jongwu commented May 6, 2019

jongwu commented May 8, 2019

grahamwhaley commented May 8, 2019

Pennyzct commented May 8, 2019

grahamwhaley commented May 8, 2019

jodh-intel commented May 8, 2019

grahamwhaley commented May 8, 2019

chavafg commented May 8, 2019

jodh-intel left a comment

jodh-intel commented May 10, 2019

jongwu commented May 13, 2019

jongwu commented May 14, 2019

grahamwhaley commented May 14, 2019

jongwu commented May 14, 2019

jcvenegas May 14, 2019

jcvenegas May 14, 2019

jongwu May 15, 2019

jodh-intel May 15, 2019

jongwu May 15, 2019

grahamwhaley May 15, 2019

grahamwhaley May 15, 2019

Pennyzct May 15, 2019

jongwu May 15, 2019

Pennyzct commented May 16, 2019

grahamwhaley commented May 16, 2019

jongwu commented May 17, 2019

devimc commented May 17, 2019


		var pciBusRescanFile = sysfsDir + "/bus/pci/rescan"

		func rescanPciBus() error {

updateInterface: enable hot-add nic on arm64 #545

updateInterface: enable hot-add nic on arm64 #545

Conversation

jongwu commented Apr 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teawater commented May 1, 2019

jongwu commented May 6, 2019

jongwu commented May 8, 2019

grahamwhaley commented May 8, 2019

Pennyzct commented May 8, 2019

grahamwhaley commented May 8, 2019

jodh-intel commented May 8, 2019

grahamwhaley commented May 8, 2019

chavafg commented May 8, 2019

jodh-intel left a comment

Choose a reason for hiding this comment

jodh-intel commented May 10, 2019

jongwu commented May 13, 2019

jongwu commented May 14, 2019

grahamwhaley commented May 14, 2019

jongwu commented May 14, 2019

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

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

Choose a reason for hiding this comment

Pennyzct commented May 16, 2019

grahamwhaley commented May 16, 2019

jongwu commented May 17, 2019

devimc commented May 17, 2019

jongwu commented Apr 30, 2019 •

edited

Loading