-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SR-IOV CNI plugin #259
Conversation
@@ -0,0 +1,54 @@ | |||
# SR-IOV CNI plugin | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a bit empty here! Could you add an introduction to this plugin that explains what it does and what it's used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introduction added:)
|
||
``` | ||
# vi /etc/modprobe.conf | ||
options ixgbe max_vfs=8,8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a requirement for the plugin to work or just an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's requirement. SR-IOV plugin need NIC enable SR-IOV capability, which not enabled by default.
In general this plugin looks fine and I'm in favor of merging this PR. A couple of notes:
|
This plugin make container work with NIC with SR-IOV capabilities.
@steveej I add some tests, but didn't enable it by default. You can run it with SR-IOV NIC. Also, you can run example in docment. Any problems let me know please:) |
@steveej @hustcat I'm not sure how I feel about us having plugins with tests that we can't actually run in CI. I think that us maintaining the plugin in the repo implies a certain level of support and endorsement, which I expect to be backed-up by test coverage running in our CI. Perhaps such plugins may be better maintained by 3rd parties. One way to address this could be to maintain a list in our readme, pointing to 3rd party plugins living in other repos. So there can be some level of information sharing and social capital, without us taking on the burden of maintaining those plugins Thoughts? @containernetworking/cni-maintainers |
Created a standalone repo here. |
@hustcat That README PR has been merged -- so your plugin repo is now linked to from the main page. I'm going to close this PR now. Feel free to re-open if you'd like us to re-consider your plugin for merging into this repo, but then I'd ask that we figure out how to run the test suite in our CI. |
Add SR-IOV CNI plugin. #netlink needed to be bump to the latest.