Skip to content
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 support for modifying iov settings in ncproxy #964

Merged
merged 1 commit into from
Apr 1, 2021

Conversation

katiewasnothere
Copy link
Contributor

@katiewasnothere katiewasnothere commented Mar 10, 2021

This PR adds the ability to modify a network through ncproxy, particularly for disabling/enabling IOV on an endpoint. Thanks to @dcantah for writing a good chunk of the ModifyNIC flow.

Currently we require that the caller passes in all values related to IOV settings. This is because there is a discrepancy between how HCS and HCN treats unset values, see comment in discussion.

Signed-off-by: Kathryn Baldauf kabaldau@microsoft.com

@katiewasnothere katiewasnothere requested a review from a team as a code owner March 10, 2021 21:20
cmd/ncproxy/ncproxy.go Outdated Show resolved Hide resolved
internal/uvm/network.go Outdated Show resolved Hide resolved
@katiewasnothere katiewasnothere force-pushed the ncproxy-iov branch 3 times, most recently from 54bdbf5 to c176d66 Compare March 16, 2021 19:05
@katiewasnothere
Copy link
Contributor Author

@microsoft/containerplat I was able to validate this again :)

cmd/ncproxy/ncproxy.go Outdated Show resolved Hide resolved
cmd/ncproxy/ncproxy.go Outdated Show resolved Hide resolved
@dcantah
Copy link
Contributor

dcantah commented Mar 29, 2021

This needs a rebase I believe, it's still showing the appveyor ci

cmd/ncproxy/ncproxy.go Outdated Show resolved Hide resolved
cmd/ncproxy/ncproxy.go Outdated Show resolved Hide resolved
@dcantah
Copy link
Contributor

dcantah commented Mar 29, 2021

Changes lgtm besides the logging/nits, but don't want to lgtm as I forget if it resets on push now with our new settings

@dcantah
Copy link
Contributor

dcantah commented Mar 29, 2021

Can take the TODO out of the PR description now also as you re-validated

@katiewasnothere katiewasnothere force-pushed the ncproxy-iov branch 2 times, most recently from de5a934 to 463f292 Compare March 29, 2021 20:47
Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>
Copy link
Contributor

@dcantah dcantah left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants