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

Cleanup network to make sure physical interfaces are restores back to original host driver. #8647

Merged
merged 2 commits into from Feb 20, 2024

Conversation

amshinde
Copy link
Member

network: Move up defer block to cleanup network

Move the defer for cleaning up network before the call to add network.
This way if any change made by add network is reverted by in case of
failure. This is particulary important for physical network interfaces
as with this step we make sure that driver for the physical interface is
reverted back to the original host driver. Without this the physical
network iterface will remain bound to vfio.

Fixes: #8646

@amshinde amshinde requested a review from cmaf December 13, 2023 05:21
@katacontainersbot katacontainersbot added the size/small Small and simple task label Dec 13, 2023
@amshinde amshinde added do-not-merge PR has problems or depends on another and removed size/small Small and simple task labels Dec 13, 2023
@amshinde
Copy link
Member Author

Adding dnm label for now as I need to some more testing in case errors are encountered in other code paths such as error in CreateContainer.
Would appreciate some feedback till then.

Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

Thanks @amshinde.

lgtm

Copy link
Contributor

@dborquez dborquez left a comment

Choose a reason for hiding this comment

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

lgtm

@amshinde
Copy link
Member Author

/test

@amshinde amshinde removed the do-not-merge PR has problems or depends on another label Jan 11, 2024
@amshinde
Copy link
Member Author

/test

@amshinde
Copy link
Member Author

amshinde commented Feb 8, 2024

/test

@katacontainersbot katacontainersbot added the size/small Small and simple task label Feb 8, 2024
@amshinde amshinde force-pushed the cleanup-network branch 2 times, most recently from 7248ec8 to fc9a40b Compare February 15, 2024 15:57
Move the defer for cleaning up network before the call to add network.
This way if any change made by add network is reverted by in case of
failure. This is particulary important for physical network interfaces
as with this step we make sure that driver for the physical interface is
reverted back to the original host driver. Without this the physical
network iterface will remain bound to vfio.

Fixes: kata-containers#8646

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
In case an error is encountered while removing a network endpoint during
network cleanup, we cuurently return immediately with the error.
With this change, in case of error we simply log the error and proceed
towards removing the next endpoint. With this, we can cleanup the
network changes made by the shim as much as possible.
This is especially important when multiple interfaces are passed to the
network namespace using a network plugin like multus.

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
@amshinde
Copy link
Member Author

/test

@amshinde amshinde merged commit 6d84fe3 into kata-containers:main Feb 20, 2024
281 of 295 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/small Small and simple task
Projects
None yet
4 participants